From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id D192CEB64DD for ; Wed, 9 Aug 2023 09:21:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 19B9E6B0074; Wed, 9 Aug 2023 05:21:11 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 14A878E0002; Wed, 9 Aug 2023 05:21:11 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F2DFE8E0001; Wed, 9 Aug 2023 05:21:10 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id E4B646B0074 for ; Wed, 9 Aug 2023 05:21:10 -0400 (EDT) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 78817801E6 for ; Wed, 9 Aug 2023 09:21:10 +0000 (UTC) X-FDA: 81104022300.23.33D414A Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by imf25.hostedemail.com (Postfix) with ESMTP id 3D078A000E for ; Wed, 9 Aug 2023 09:21:06 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=gPn+0eHI; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=cTmN2Y4E; dmarc=none; spf=pass (imf25.hostedemail.com: domain of jack@suse.cz designates 195.135.220.28 as permitted sender) smtp.mailfrom=jack@suse.cz ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1691572867; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=sa/Ur4Xvab0nuX/5llhwwRSNuVsSYXGazw2MR1iK1sg=; b=j7JSHMh1QdP0pQatT8Ku2nXOS2VKDdjk+xbnFuYZqRhHKToTaRT595zto3pYzPBXzVoJna MGsh33g8G9VB3API2lKsv6+78NxiYuADNoj3VkJsTsSalyQTVTGM8uo3BpphD3E3AUx8x9 w+adBEl1awLkjfKooIi4yF4KCGcS4tI= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=suse.cz header.s=susede2_rsa header.b=gPn+0eHI; dkim=pass header.d=suse.cz header.s=susede2_ed25519 header.b=cTmN2Y4E; dmarc=none; spf=pass (imf25.hostedemail.com: domain of jack@suse.cz designates 195.135.220.28 as permitted sender) smtp.mailfrom=jack@suse.cz ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1691572867; a=rsa-sha256; cv=none; b=2Vt828MV5gliOB2TUiP0QM+4bqWZpdKVX19EofQwvTQ1Joa14xuusemYbqUbiH8G48H5fa iU21st55vN/IkEj4DsqNK22lgMUwKGVKfbN5I5HutpBL1S9trVca5ccB00NIxn/Z1gKzVe t6qF8T2nrYeCIUp3s3VhOaiqfFK+yLo= Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id CA0662184F; Wed, 9 Aug 2023 09:21:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1691572865; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=sa/Ur4Xvab0nuX/5llhwwRSNuVsSYXGazw2MR1iK1sg=; b=gPn+0eHIR3aHvDBuhDFyupFCpo4iecxgslyfGtGaqwipUT8GwGxqlefNMvVW7GmxA6GebR XIayPflXzfVmKgRyX+hqg2xheJTQNe3JES4cjHythwT/ZJCGxpuOAoYPyr583uyOGGufh3 OoJKoNL6vf7KLXKVqaRq+ra5BziLl5o= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1691572865; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=sa/Ur4Xvab0nuX/5llhwwRSNuVsSYXGazw2MR1iK1sg=; b=cTmN2Y4EQpWzAkVq61Jzr1Dh27eDAVl/eU1XLLtdn0n7PX2VfaVCRlLL9BE7cAXz/s4RnS aGQKounQCioLU4Bw== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 7A6D7133B5; Wed, 9 Aug 2023 09:21:05 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id SyvaHYFa02R9DgAAMHmgww (envelope-from ); Wed, 09 Aug 2023 09:21:05 +0000 Received: by quack3.suse.cz (Postfix, from userid 1000) id A4916A0769; Wed, 9 Aug 2023 11:21:04 +0200 (CEST) Date: Wed, 9 Aug 2023 11:21:04 +0200 From: Jan Kara To: Hugh Dickins Cc: Christian Brauner , Andrew Morton , Oleksandr Tymoshenko , Carlos Maiolino , Jeff Layton , Chuck Lever , Jan Kara , Miklos Szeredi , Daniel Xu , Chris Down , Tejun Heo , Greg Kroah-Hartman , Matthew Wilcox , Christoph Hellwig , Pete Zaitcev , Helge Deller , Topi Miettinen , Yu Kuai , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH vfs.tmpfs 1/5] xattr: simple_xattr_set() return old_xattr to be freed Message-ID: <20230809092104.paz5wu5m5b6blndo@quack3> References: <158c6585-2aa7-d4aa-90ff-f7c3f8fe407c@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <158c6585-2aa7-d4aa-90ff-f7c3f8fe407c@google.com> X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 3D078A000E X-Stat-Signature: ccxmimzwdfjtmnuu13t3q31mcm89omfw X-HE-Tag: 1691572866-191470 X-HE-Meta: U2FsdGVkX1/NdhdOtUV1uNzumRMeIMO5ypiS6TdOAYve58ymRHuWGSzmjqzW6L4gq0CsR71wUfS66967EgDA8U0SViqaVN6RrgDrePcXQoei617464Jk2FYmeglb/Bu6AhNs4bFp5ivjsxR7STaBAMYP8STkUNnvhjFbtUP8wUHw3OYLNbXLbY26CGCArqBLhKW0uZbnjxFHX2GyUolGn/Cjuvb1MzFq0D+9a+mjTNNAndC7VLNfJN4gW+vWh68i5RnT5Sx0gBv1Vc2yZ9GLLaGZ7ddLnywq9UXL0D/2fqG8X2ekhGLF1fN4Y+Gw3sUKeY+/OXtAEQeRu0g/WCI6NaRaM4S+RUHk+hMB0WFUwF9jtco7OqZ+JlgwMLb9OCnCYIgsX32JT0TEq1R2pvTiH1oNojRXSjDIbPK9R73S6lNKY9x60eXVCoo/gqxvk08JNut5DzQ+/5YPR11QLV/Szhvkhzqx+sbXVj04jgO9dd77M4OBYpTptcYDizLN4P/0ggp4mQkvfKphDFc+Wq3O7LCWqEHWhmRqbi53rlO/Eq8jkUdOYBcifdFhG79joN/j01dUFQcqQrvhL0xTn8DwVHEg+/dgGUKyr/c2WM3uvf2I3SfhXkc8eNRU2QkiSvU0pgZKUJnLiaTnEPHjOyFWAk7Hk+xiBQBv9/E+1rWfCmfrdaHGl76F1CQAZVc/Ob58ao67PxBnyPE2NGtCDQTuRDX6EgmLnlqfR+5OurtbX5BhTl9b4S4z/UVTWTh1HHE5Ti9mRa+WMR81R+vgzD1fsbrF6ny6LhGUDJAyAulMwaUmwmug+oVOgk/PXoa2PxLkaE+S5fMkUXtS1JdFpzg5Vw/5jvu9onezjuaC+vEqQDD9CAG7oLHUnqGQ1ahuD0HD77E6kjRP2O2czbxKZ8e0nA7XEr9Thoc9lJ8RVeivNd/FHFByffYWwuTMk17993YK0w80A9QxHn35mx55QF+ jRX1HKjX LCdHsTNYEdTaiuWV/bIGbuDbzB0Kpe+2Q5u/8QCYiIHJrbRB/D2TYwzP/0hhkLB3Rhg6pOlG6Ier/G7L8furIc6moxy0DJ0k0UNiWrlhV3EeryO2lXwXH0jb1KW++3RAykqBQobBaHM6RLbSoUYcn7FEBr0Qa/BnPbaXci5hxxz6yY9Ivy6ppW1MlWerlygjWHYiVmrzFAACcV+Zi3TsF1VSAKot7RkqHQixKq6pqxY9powvJfYlABNzJwwEm2DiQrJiLfZkME7JMP+eC6/DFAEA2e0dSFjVoKn2SEVpCGIH5E8xmoLxmDpWBSFdWyfNr/faTd55kJucVHykzH0M29qFkEeBloXe5D3+E X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue 08-08-23 21:30:59, Hugh Dickins wrote: > 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 Looks good to me. Feel free to add: Reviewed-by: Jan Kara Honza > --- > fs/kernfs/inode.c | 46 +++++++++++++++++++++++++--------------- > fs/xattr.c | 51 +++++++++++++++++++-------------------------- > include/linux/xattr.h | 7 ++++--- > mm/shmem.c | 10 +++++---- > 4 files changed, 61 insertions(+), 53 deletions(-) > > diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c > index b22b74d1a115..fec5d5f78f07 100644 > --- a/fs/kernfs/inode.c > +++ b/fs/kernfs/inode.c > @@ -306,11 +306,17 @@ int kernfs_xattr_get(struct kernfs_node *kn, const char *name, > int kernfs_xattr_set(struct kernfs_node *kn, const char *name, > const void *value, size_t size, int flags) > { > + struct simple_xattr *old_xattr; > struct kernfs_iattrs *attrs = kernfs_iattrs(kn); > if (!attrs) > return -ENOMEM; > > - return simple_xattr_set(&attrs->xattrs, name, value, size, flags, NULL); > + old_xattr = simple_xattr_set(&attrs->xattrs, name, value, size, flags); > + if (IS_ERR(old_xattr)) > + return PTR_ERR(old_xattr); > + > + simple_xattr_free(old_xattr); > + return 0; > } > > 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, > { > atomic_t *sz = &kn->iattr->user_xattr_size; > atomic_t *nr = &kn->iattr->nr_user_xattrs; > - ssize_t removed_size; > + struct simple_xattr *old_xattr; > int ret; > > if (atomic_inc_return(nr) > KERNFS_MAX_USER_XATTRS) { > @@ -355,13 +361,18 @@ static int kernfs_vfs_user_xattr_add(struct kernfs_node *kn, > goto dec_size_out; > } > > - ret = simple_xattr_set(xattrs, full_name, value, size, flags, > - &removed_size); > - > - if (!ret && removed_size >= 0) > - size = removed_size; > - else if (!ret) > + old_xattr = simple_xattr_set(xattrs, full_name, value, size, flags); > + if (!old_xattr) > return 0; > + > + if (IS_ERR(old_xattr)) { > + ret = PTR_ERR(old_xattr); > + goto dec_size_out; > + } > + > + ret = 0; > + size = old_xattr->size; > + simple_xattr_free(old_xattr); > dec_size_out: > atomic_sub(size, sz); > dec_count_out: > @@ -376,18 +387,19 @@ static int kernfs_vfs_user_xattr_rm(struct kernfs_node *kn, > { > atomic_t *sz = &kn->iattr->user_xattr_size; > atomic_t *nr = &kn->iattr->nr_user_xattrs; > - ssize_t removed_size; > - int ret; > + struct simple_xattr *old_xattr; > > - ret = simple_xattr_set(xattrs, full_name, value, size, flags, > - &removed_size); > + old_xattr = simple_xattr_set(xattrs, full_name, value, size, flags); > + if (!old_xattr) > + return 0; > > - if (removed_size >= 0) { > - atomic_sub(removed_size, sz); > - atomic_dec(nr); > - } > + if (IS_ERR(old_xattr)) > + return PTR_ERR(old_xattr); > > - return ret; > + atomic_sub(old_xattr->size, sz); > + atomic_dec(nr); > + simple_xattr_free(old_xattr); > + return 0; > } > > static int kernfs_vfs_user_xattr_set(const struct xattr_handler *handler, > diff --git a/fs/xattr.c b/fs/xattr.c > index e7bbb7f57557..ba37a8f5cfd1 100644 > --- a/fs/xattr.c > +++ b/fs/xattr.c > @@ -1040,12 +1040,12 @@ const char *xattr_full_name(const struct xattr_handler *handler, > EXPORT_SYMBOL(xattr_full_name); > > /** > - * free_simple_xattr - free an xattr object > + * simple_xattr_free - free an xattr object > * @xattr: the xattr object > * > * Free the xattr object. Can handle @xattr being NULL. > */ > -static inline void free_simple_xattr(struct simple_xattr *xattr) > +void simple_xattr_free(struct simple_xattr *xattr) > { > if (xattr) > kfree(xattr->name); > @@ -1164,7 +1164,6 @@ int simple_xattr_get(struct simple_xattrs *xattrs, const char *name, > * @value: the value to store along the xattr > * @size: the size of @value > * @flags: the flags determining how to set the xattr > - * @removed_size: the size of the removed xattr > * > * Set a new xattr object. > * 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, > * nothing if XATTR_CREATE is specified in @flags or @flags is zero. For > * XATTR_REPLACE we fail as mentioned above. > * > - * Return: On success zero and on error a negative error code is returned. > + * Return: On success, the removed or replaced xattr is returned, to be freed > + * by the caller; or NULL if none. On failure a negative error code is returned. > */ > -int simple_xattr_set(struct simple_xattrs *xattrs, const char *name, > - const void *value, size_t size, int flags, > - ssize_t *removed_size) > +struct simple_xattr *simple_xattr_set(struct simple_xattrs *xattrs, > + const char *name, const void *value, > + size_t size, int flags) > { > - struct simple_xattr *xattr = NULL, *new_xattr = NULL; > + struct simple_xattr *old_xattr = NULL, *new_xattr = NULL; > struct rb_node *parent = NULL, **rbp; > int err = 0, ret; > > - if (removed_size) > - *removed_size = -1; > - > /* value == NULL means remove */ > if (value) { > new_xattr = simple_xattr_alloc(value, size); > if (!new_xattr) > - return -ENOMEM; > + return ERR_PTR(-ENOMEM); > > new_xattr->name = kstrdup(name, GFP_KERNEL); > if (!new_xattr->name) { > - free_simple_xattr(new_xattr); > - return -ENOMEM; > + simple_xattr_free(new_xattr); > + return ERR_PTR(-ENOMEM); > } > } > > @@ -1217,12 +1214,12 @@ int simple_xattr_set(struct simple_xattrs *xattrs, const char *name, > else if (ret > 0) > rbp = &(*rbp)->rb_right; > else > - xattr = rb_entry(*rbp, struct simple_xattr, rb_node); > - if (xattr) > + old_xattr = rb_entry(*rbp, struct simple_xattr, rb_node); > + if (old_xattr) > break; > } > > - if (xattr) { > + if (old_xattr) { > /* Fail if XATTR_CREATE is requested and the xattr exists. */ > if (flags & XATTR_CREATE) { > err = -EEXIST; > @@ -1230,12 +1227,10 @@ int simple_xattr_set(struct simple_xattrs *xattrs, const char *name, > } > > if (new_xattr) > - rb_replace_node(&xattr->rb_node, &new_xattr->rb_node, > - &xattrs->rb_root); > + rb_replace_node(&old_xattr->rb_node, > + &new_xattr->rb_node, &xattrs->rb_root); > else > - rb_erase(&xattr->rb_node, &xattrs->rb_root); > - if (!err && removed_size) > - *removed_size = xattr->size; > + rb_erase(&old_xattr->rb_node, &xattrs->rb_root); > } else { > /* Fail if XATTR_REPLACE is requested but no xattr is found. */ > if (flags & XATTR_REPLACE) { > @@ -1260,12 +1255,10 @@ int simple_xattr_set(struct simple_xattrs *xattrs, const char *name, > > out_unlock: > write_unlock(&xattrs->lock); > - if (err) > - free_simple_xattr(new_xattr); > - else > - free_simple_xattr(xattr); > - return err; > - > + if (!err) > + return old_xattr; > + simple_xattr_free(new_xattr); > + return ERR_PTR(err); > } > > static bool xattr_is_trusted(const char *name) > @@ -1386,7 +1379,7 @@ void simple_xattrs_free(struct simple_xattrs *xattrs) > rbp_next = rb_next(rbp); > xattr = rb_entry(rbp, struct simple_xattr, rb_node); > rb_erase(&xattr->rb_node, &xattrs->rb_root); > - free_simple_xattr(xattr); > + simple_xattr_free(xattr); > rbp = rbp_next; > } > } > diff --git a/include/linux/xattr.h b/include/linux/xattr.h > index d591ef59aa98..e37fe667ae04 100644 > --- a/include/linux/xattr.h > +++ b/include/linux/xattr.h > @@ -116,11 +116,12 @@ struct simple_xattr { > void simple_xattrs_init(struct simple_xattrs *xattrs); > void simple_xattrs_free(struct simple_xattrs *xattrs); > struct simple_xattr *simple_xattr_alloc(const void *value, size_t size); > +void simple_xattr_free(struct simple_xattr *xattr); > int simple_xattr_get(struct simple_xattrs *xattrs, const char *name, > void *buffer, size_t size); > -int simple_xattr_set(struct simple_xattrs *xattrs, const char *name, > - const void *value, size_t size, int flags, > - ssize_t *removed_size); > +struct simple_xattr *simple_xattr_set(struct simple_xattrs *xattrs, > + const char *name, const void *value, > + size_t size, int flags); > ssize_t simple_xattr_list(struct inode *inode, struct simple_xattrs *xattrs, > char *buffer, size_t size); > void simple_xattr_add(struct simple_xattrs *xattrs, > diff --git a/mm/shmem.c b/mm/shmem.c > index 0f83d86fd8b4..df3cabf54206 100644 > --- a/mm/shmem.c > +++ b/mm/shmem.c > @@ -3595,15 +3595,17 @@ static int shmem_xattr_handler_set(const struct xattr_handler *handler, > size_t size, int flags) > { > struct shmem_inode_info *info = SHMEM_I(inode); > - int err; > + struct simple_xattr *old_xattr; > > name = xattr_full_name(handler, name); > - err = simple_xattr_set(&info->xattrs, name, value, size, flags, NULL); > - if (!err) { > + old_xattr = simple_xattr_set(&info->xattrs, name, value, size, flags); > + if (!IS_ERR(old_xattr)) { > + simple_xattr_free(old_xattr); > + old_xattr = NULL; > inode->i_ctime = current_time(inode); > inode_inc_iversion(inode); > } > - return err; > + return PTR_ERR(old_xattr); > } > > static const struct xattr_handler shmem_security_xattr_handler = { > -- > 2.35.3 > -- Jan Kara SUSE Labs, CR