Skip to content

Commit 5de7597

Browse files
Hugh Dickinsbrauner
Hugh Dickins
authored and
brauner
committed
xattr: simple_xattr_set() return old_xattr to be freed
tmpfs wants to support limited user extended attributes, but kernfs (or cgroupfs, the only kernfs with KERNFS_ROOT_SUPPORT_USER_XATTR) already supports user extended attributes through simple xattrs: but limited by a policy (128KiB per inode) too liberal to be used on tmpfs. To allow a different limiting policy for tmpfs, without affecting the policy for kernfs, change simple_xattr_set() to return the replaced or removed xattr (if any), leaving the caller to update their accounting then free the xattr (by simple_xattr_free(), renamed from the static free_simple_xattr()). Signed-off-by: Hugh Dickins <[email protected]> Reviewed-by: Jan Kara <[email protected]> Reviewed-by: Christian Brauner <[email protected]> Reviewed-by: Carlos Maiolino <[email protected]> Message-Id: <[email protected]> Signed-off-by: Christian Brauner <[email protected]>
1 parent 0200679 commit 5de7597

File tree

4 files changed

+61
-53
lines changed

4 files changed

+61
-53
lines changed

‎fs/kernfs/inode.c

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -306,11 +306,17 @@ int kernfs_xattr_get(struct kernfs_node *kn, const char *name,
306306
int kernfs_xattr_set(struct kernfs_node *kn, const char *name,
307307
const void *value, size_t size, int flags)
308308
{
309+
struct simple_xattr *old_xattr;
309310
struct kernfs_iattrs *attrs = kernfs_iattrs(kn);
310311
if (!attrs)
311312
return -ENOMEM;
312313

313-
return simple_xattr_set(&attrs->xattrs, name, value, size, flags, NULL);
314+
old_xattr = simple_xattr_set(&attrs->xattrs, name, value, size, flags);
315+
if (IS_ERR(old_xattr))
316+
return PTR_ERR(old_xattr);
317+
318+
simple_xattr_free(old_xattr);
319+
return 0;
314320
}
315321

316322
static int kernfs_vfs_xattr_get(const struct xattr_handler *handler,
@@ -342,7 +348,7 @@ static int kernfs_vfs_user_xattr_add(struct kernfs_node *kn,
342348
{
343349
atomic_t *sz = &kn->iattr->user_xattr_size;
344350
atomic_t *nr = &kn->iattr->nr_user_xattrs;
345-
ssize_t removed_size;
351+
struct simple_xattr *old_xattr;
346352
int ret;
347353

348354
if (atomic_inc_return(nr) > KERNFS_MAX_USER_XATTRS) {
@@ -355,13 +361,18 @@ static int kernfs_vfs_user_xattr_add(struct kernfs_node *kn,
355361
goto dec_size_out;
356362
}
357363

358-
ret = simple_xattr_set(xattrs, full_name, value, size, flags,
359-
&removed_size);
360-
361-
if (!ret && removed_size >= 0)
362-
size = removed_size;
363-
else if (!ret)
364+
old_xattr = simple_xattr_set(xattrs, full_name, value, size, flags);
365+
if (!old_xattr)
364366
return 0;
367+
368+
if (IS_ERR(old_xattr)) {
369+
ret = PTR_ERR(old_xattr);
370+
goto dec_size_out;
371+
}
372+
373+
ret = 0;
374+
size = old_xattr->size;
375+
simple_xattr_free(old_xattr);
365376
dec_size_out:
366377
atomic_sub(size, sz);
367378
dec_count_out:
@@ -376,18 +387,19 @@ static int kernfs_vfs_user_xattr_rm(struct kernfs_node *kn,
376387
{
377388
atomic_t *sz = &kn->iattr->user_xattr_size;
378389
atomic_t *nr = &kn->iattr->nr_user_xattrs;
379-
ssize_t removed_size;
380-
int ret;
390+
struct simple_xattr *old_xattr;
381391

382-
ret = simple_xattr_set(xattrs, full_name, value, size, flags,
383-
&removed_size);
392+
old_xattr = simple_xattr_set(xattrs, full_name, value, size, flags);
393+
if (!old_xattr)
394+
return 0;
384395

385-
if (removed_size >= 0) {
386-
atomic_sub(removed_size, sz);
387-
atomic_dec(nr);
388-
}
396+
if (IS_ERR(old_xattr))
397+
return PTR_ERR(old_xattr);
389398

390-
return ret;
399+
atomic_sub(old_xattr->size, sz);
400+
atomic_dec(nr);
401+
simple_xattr_free(old_xattr);
402+
return 0;
391403
}
392404

393405
static int kernfs_vfs_user_xattr_set(const struct xattr_handler *handler,

‎fs/xattr.c

Lines changed: 22 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1040,12 +1040,12 @@ const char *xattr_full_name(const struct xattr_handler *handler,
10401040
EXPORT_SYMBOL(xattr_full_name);
10411041

10421042
/**
1043-
* free_simple_xattr - free an xattr object
1043+
* simple_xattr_free - free an xattr object
10441044
* @xattr: the xattr object
10451045
*
10461046
* Free the xattr object. Can handle @xattr being NULL.
10471047
*/
1048-
static inline void free_simple_xattr(struct simple_xattr *xattr)
1048+
void simple_xattr_free(struct simple_xattr *xattr)
10491049
{
10501050
if (xattr)
10511051
kfree(xattr->name);
@@ -1164,7 +1164,6 @@ int simple_xattr_get(struct simple_xattrs *xattrs, const char *name,
11641164
* @value: the value to store along the xattr
11651165
* @size: the size of @value
11661166
* @flags: the flags determining how to set the xattr
1167-
* @removed_size: the size of the removed xattr
11681167
*
11691168
* Set a new xattr object.
11701169
* If @value is passed a new xattr object will be allocated. If XATTR_REPLACE
@@ -1181,29 +1180,27 @@ int simple_xattr_get(struct simple_xattrs *xattrs, const char *name,
11811180
* nothing if XATTR_CREATE is specified in @flags or @flags is zero. For
11821181
* XATTR_REPLACE we fail as mentioned above.
11831182
*
1184-
* Return: On success zero and on error a negative error code is returned.
1183+
* Return: On success, the removed or replaced xattr is returned, to be freed
1184+
* by the caller; or NULL if none. On failure a negative error code is returned.
11851185
*/
1186-
int simple_xattr_set(struct simple_xattrs *xattrs, const char *name,
1187-
const void *value, size_t size, int flags,
1188-
ssize_t *removed_size)
1186+
struct simple_xattr *simple_xattr_set(struct simple_xattrs *xattrs,
1187+
const char *name, const void *value,
1188+
size_t size, int flags)
11891189
{
1190-
struct simple_xattr *xattr = NULL, *new_xattr = NULL;
1190+
struct simple_xattr *old_xattr = NULL, *new_xattr = NULL;
11911191
struct rb_node *parent = NULL, **rbp;
11921192
int err = 0, ret;
11931193

1194-
if (removed_size)
1195-
*removed_size = -1;
1196-
11971194
/* value == NULL means remove */
11981195
if (value) {
11991196
new_xattr = simple_xattr_alloc(value, size);
12001197
if (!new_xattr)
1201-
return -ENOMEM;
1198+
return ERR_PTR(-ENOMEM);
12021199

12031200
new_xattr->name = kstrdup(name, GFP_KERNEL);
12041201
if (!new_xattr->name) {
1205-
free_simple_xattr(new_xattr);
1206-
return -ENOMEM;
1202+
simple_xattr_free(new_xattr);
1203+
return ERR_PTR(-ENOMEM);
12071204
}
12081205
}
12091206

@@ -1217,25 +1214,23 @@ int simple_xattr_set(struct simple_xattrs *xattrs, const char *name,
12171214
else if (ret > 0)
12181215
rbp = &(*rbp)->rb_right;
12191216
else
1220-
xattr = rb_entry(*rbp, struct simple_xattr, rb_node);
1221-
if (xattr)
1217+
old_xattr = rb_entry(*rbp, struct simple_xattr, rb_node);
1218+
if (old_xattr)
12221219
break;
12231220
}
12241221

1225-
if (xattr) {
1222+
if (old_xattr) {
12261223
/* Fail if XATTR_CREATE is requested and the xattr exists. */
12271224
if (flags & XATTR_CREATE) {
12281225
err = -EEXIST;
12291226
goto out_unlock;
12301227
}
12311228

12321229
if (new_xattr)
1233-
rb_replace_node(&xattr->rb_node, &new_xattr->rb_node,
1234-
&xattrs->rb_root);
1230+
rb_replace_node(&old_xattr->rb_node,
1231+
&new_xattr->rb_node, &xattrs->rb_root);
12351232
else
1236-
rb_erase(&xattr->rb_node, &xattrs->rb_root);
1237-
if (!err && removed_size)
1238-
*removed_size = xattr->size;
1233+
rb_erase(&old_xattr->rb_node, &xattrs->rb_root);
12391234
} else {
12401235
/* Fail if XATTR_REPLACE is requested but no xattr is found. */
12411236
if (flags & XATTR_REPLACE) {
@@ -1260,12 +1255,10 @@ int simple_xattr_set(struct simple_xattrs *xattrs, const char *name,
12601255

12611256
out_unlock:
12621257
write_unlock(&xattrs->lock);
1263-
if (err)
1264-
free_simple_xattr(new_xattr);
1265-
else
1266-
free_simple_xattr(xattr);
1267-
return err;
1268-
1258+
if (!err)
1259+
return old_xattr;
1260+
simple_xattr_free(new_xattr);
1261+
return ERR_PTR(err);
12691262
}
12701263

12711264
static bool xattr_is_trusted(const char *name)
@@ -1386,7 +1379,7 @@ void simple_xattrs_free(struct simple_xattrs *xattrs)
13861379
rbp_next = rb_next(rbp);
13871380
xattr = rb_entry(rbp, struct simple_xattr, rb_node);
13881381
rb_erase(&xattr->rb_node, &xattrs->rb_root);
1389-
free_simple_xattr(xattr);
1382+
simple_xattr_free(xattr);
13901383
rbp = rbp_next;
13911384
}
13921385
}

‎include/linux/xattr.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,11 +116,12 @@ struct simple_xattr {
116116
void simple_xattrs_init(struct simple_xattrs *xattrs);
117117
void simple_xattrs_free(struct simple_xattrs *xattrs);
118118
struct simple_xattr *simple_xattr_alloc(const void *value, size_t size);
119+
void simple_xattr_free(struct simple_xattr *xattr);
119120
int simple_xattr_get(struct simple_xattrs *xattrs, const char *name,
120121
void *buffer, size_t size);
121-
int simple_xattr_set(struct simple_xattrs *xattrs, const char *name,
122-
const void *value, size_t size, int flags,
123-
ssize_t *removed_size);
122+
struct simple_xattr *simple_xattr_set(struct simple_xattrs *xattrs,
123+
const char *name, const void *value,
124+
size_t size, int flags);
124125
ssize_t simple_xattr_list(struct inode *inode, struct simple_xattrs *xattrs,
125126
char *buffer, size_t size);
126127
void simple_xattr_add(struct simple_xattrs *xattrs,

‎mm/shmem.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3598,15 +3598,17 @@ static int shmem_xattr_handler_set(const struct xattr_handler *handler,
35983598
size_t size, int flags)
35993599
{
36003600
struct shmem_inode_info *info = SHMEM_I(inode);
3601-
int err;
3601+
struct simple_xattr *old_xattr;
36023602

36033603
name = xattr_full_name(handler, name);
3604-
err = simple_xattr_set(&info->xattrs, name, value, size, flags, NULL);
3605-
if (!err) {
3604+
old_xattr = simple_xattr_set(&info->xattrs, name, value, size, flags);
3605+
if (!IS_ERR(old_xattr)) {
3606+
simple_xattr_free(old_xattr);
3607+
old_xattr = NULL;
36063608
inode->i_ctime = current_time(inode);
36073609
inode_inc_iversion(inode);
36083610
}
3609-
return err;
3611+
return PTR_ERR(old_xattr);
36103612
}
36113613

36123614
static const struct xattr_handler shmem_security_xattr_handler = {

0 commit comments

Comments
 (0)