From: Dave Chinner <david@fromorbit.com> To: Artem Savkov <asavkov@redhat.com> Cc: Eric Sandeen <sandeen@sandeen.net>, xfs@oss.sgi.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH] Make __xfs_xattr_put_listen preperly report errors. Date: Wed, 24 Aug 2016 11:55:51 +1000 [thread overview] Message-ID: <20160824015551.GB19025@dastard> (raw) In-Reply-To: <1471967653-2561-1-git-send-email-asavkov@redhat.com> On Tue, Aug 23, 2016 at 05:54:13PM +0200, Artem Savkov wrote: > Commit "xfs: only return -errno or success from attr ->put_listent" changes the Please quote commits in --oneline format in changelogs - it makes it much easier to find the change you are refering to if there is both a commit ID and the text string in the commit message. (i.e. text string confirms the commit id is the one you meant to quote). commit 2a6fba6 ("xfs: only return -errno or success from attr ->put_listent") is the one you are refering to here, right? > returnvalue of __xfs_xattr_put_listen to 0 in case when there is insufficient > space in the buffer assuming that setting context->count to -1 would be enough, > but all of the ->put_listent callers only check seen_enough. This results in > a failed assertion: > XFS: Assertion failed: context->count >= 0, file: fs/xfs/xfs_xattr.c, line: 175 > in insufficient buffer size case. You have a test case? Can you turn it into an xfstest? We really need regression tests that cover issues like this.... > Signed-off-by: Artem Savkov <asavkov@redhat.com> > --- > fs/xfs/xfs_xattr.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c > index ea62245..6290093 100644 > --- a/fs/xfs/xfs_xattr.c > +++ b/fs/xfs/xfs_xattr.c > @@ -147,6 +147,7 @@ __xfs_xattr_put_listent( > arraytop = context->count + prefix_len + namelen + 1; > if (arraytop > context->firstu) { > context->count = -1; /* insufficient space */ > + context->seen_enough = 1; > return 0; > } > offset = (char *)context->alist + context->count; Looks sane, though I don't know how to test it yet.... Cheers, Dave. -- Dave Chinner david@fromorbit.com
WARNING: multiple messages have this Message-ID (diff)
From: Dave Chinner <david@fromorbit.com> To: Artem Savkov <asavkov@redhat.com> Cc: Eric Sandeen <sandeen@sandeen.net>, linux-kernel@vger.kernel.org, xfs@oss.sgi.com Subject: Re: [PATCH] Make __xfs_xattr_put_listen preperly report errors. Date: Wed, 24 Aug 2016 11:55:51 +1000 [thread overview] Message-ID: <20160824015551.GB19025@dastard> (raw) In-Reply-To: <1471967653-2561-1-git-send-email-asavkov@redhat.com> On Tue, Aug 23, 2016 at 05:54:13PM +0200, Artem Savkov wrote: > Commit "xfs: only return -errno or success from attr ->put_listent" changes the Please quote commits in --oneline format in changelogs - it makes it much easier to find the change you are refering to if there is both a commit ID and the text string in the commit message. (i.e. text string confirms the commit id is the one you meant to quote). commit 2a6fba6 ("xfs: only return -errno or success from attr ->put_listent") is the one you are refering to here, right? > returnvalue of __xfs_xattr_put_listen to 0 in case when there is insufficient > space in the buffer assuming that setting context->count to -1 would be enough, > but all of the ->put_listent callers only check seen_enough. This results in > a failed assertion: > XFS: Assertion failed: context->count >= 0, file: fs/xfs/xfs_xattr.c, line: 175 > in insufficient buffer size case. You have a test case? Can you turn it into an xfstest? We really need regression tests that cover issues like this.... > Signed-off-by: Artem Savkov <asavkov@redhat.com> > --- > fs/xfs/xfs_xattr.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c > index ea62245..6290093 100644 > --- a/fs/xfs/xfs_xattr.c > +++ b/fs/xfs/xfs_xattr.c > @@ -147,6 +147,7 @@ __xfs_xattr_put_listent( > arraytop = context->count + prefix_len + namelen + 1; > if (arraytop > context->firstu) { > context->count = -1; /* insufficient space */ > + context->seen_enough = 1; > return 0; > } > offset = (char *)context->alist + context->count; Looks sane, though I don't know how to test it yet.... Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2016-08-24 1:57 UTC|newest] Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-08-23 15:54 [PATCH] Make __xfs_xattr_put_listen preperly report errors Artem Savkov 2016-08-23 15:54 ` Artem Savkov 2016-08-24 1:55 ` Dave Chinner [this message] 2016-08-24 1:55 ` Dave Chinner 2016-08-24 8:08 ` Artem Savkov 2016-08-24 8:08 ` Artem Savkov 2016-08-25 0:24 ` Dave Chinner 2016-08-25 0:24 ` Dave Chinner 2016-08-25 8:21 ` Artem Savkov 2016-08-25 8:21 ` Artem Savkov 2016-08-25 22:42 ` Dave Chinner 2016-08-25 22:42 ` Dave Chinner 2016-08-26 8:59 ` Artem Savkov 2016-08-26 8:59 ` Artem Savkov 2016-08-28 22:55 ` Dave Chinner 2016-08-28 22:55 ` Dave Chinner 2016-08-29 8:12 ` [PATCH v2] " Artem Savkov 2016-08-29 8:12 ` Artem Savkov 2016-08-25 21:36 ` [PATCH] " Eric Sandeen
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20160824015551.GB19025@dastard \ --to=david@fromorbit.com \ --cc=asavkov@redhat.com \ --cc=linux-kernel@vger.kernel.org \ --cc=sandeen@sandeen.net \ --cc=xfs@oss.sgi.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.