| Message ID | 20181215101329.16246-1-richard@nod.at |
|---|---|
| State | New, archived |
| Delegated to: | Richard Weinberger |
| Headers | show |
| Series | jffs2: Fix retry handling jffs2_listxattr | expand |
Hi Richard, On Sat, 15 Dec 2018 at 11:22, Richard Weinberger <richard@nod.at> wrote: > When jffs2 has to retry reading xattrs we need to reset > the buffer pointer. Otherwise we return old xattrs from the > previous iteration which leads to a inconsistency between > the number of bytes we return and the real list size. > > Cc: <stable@vger.kernel.org> > Cc: Andreas Gruenbacher <agruenba@redhat.com> > Fixes: 764a5c6b1fa4 ("xattr handlers: Simplify list operation") > Signed-off-by: Richard Weinberger <richard@nod.at> Reviewed-by: Andreas Gruenbacher <agruenba@redhat.com> > --- > Andreas, > > since you maintain the attr package too, I report it right here. :-) > This jffs2 bug lead to a crash in attr_list(). > > for() will loop to crash when there is no trailing \0 in the > list of xattrs. > > for (l = lbuf; l != lbuf + length; l = strchr(l, '\0') + 1) { > if (api_unconvert(name, l, flags)) > continue; > > ... > } > > I suggest changing the loop condition to something like l < lbuf + length. With that change alone, strchr would still go beyond the buffer. Let's just append a null character at the end instead. Thanks, Andreas > Thanks, > //richard > --- > fs/jffs2/xattr.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c > index da3e18503c65..0cb322eb9516 100644 > --- a/fs/jffs2/xattr.c > +++ b/fs/jffs2/xattr.c > @@ -967,6 +967,7 @@ ssize_t jffs2_listxattr(struct dentry *dentry, char *buffer, size_t size) > struct jffs2_xattr_ref *ref, **pref; > struct jffs2_xattr_datum *xd; > const struct xattr_handler *xhandle; > + char *orig_buffer = buffer; > const char *prefix; > ssize_t prefix_len, len, rc; > int retry = 0; > @@ -977,6 +978,7 @@ ssize_t jffs2_listxattr(struct dentry *dentry, char *buffer, size_t size) > > down_read(&c->xattr_sem); > retry: > + buffer = orig_buffer; > len = 0; > for (ref=ic->xref, pref=&ic->xref; ref; pref=&ref->next, ref=ref->next) { > BUG_ON(ref->ic != ic); > -- > 2.20.0 >
diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c index da3e18503c65..0cb322eb9516 100644 --- a/fs/jffs2/xattr.c +++ b/fs/jffs2/xattr.c @@ -967,6 +967,7 @@ ssize_t jffs2_listxattr(struct dentry *dentry, char *buffer, size_t size) struct jffs2_xattr_ref *ref, **pref; struct jffs2_xattr_datum *xd; const struct xattr_handler *xhandle; + char *orig_buffer = buffer; const char *prefix; ssize_t prefix_len, len, rc; int retry = 0; @@ -977,6 +978,7 @@ ssize_t jffs2_listxattr(struct dentry *dentry, char *buffer, size_t size) down_read(&c->xattr_sem); retry: + buffer = orig_buffer; len = 0; for (ref=ic->xref, pref=&ic->xref; ref; pref=&ref->next, ref=ref->next) { BUG_ON(ref->ic != ic);
When jffs2 has to retry reading xattrs we need to reset the buffer pointer. Otherwise we return old xattrs from the previous iteration which leads to a inconsistency between the number of bytes we return and the real list size. Cc: <stable@vger.kernel.org> Cc: Andreas Gruenbacher <agruenba@redhat.com> Fixes: 764a5c6b1fa4 ("xattr handlers: Simplify list operation") Signed-off-by: Richard Weinberger <richard@nod.at> --- Andreas, since you maintain the attr package too, I report it right here. :-) This jffs2 bug lead to a crash in attr_list(). for() will loop to crash when there is no trailing \0 in the list of xattrs. for (l = lbuf; l != lbuf + length; l = strchr(l, '\0') + 1) { if (api_unconvert(name, l, flags)) continue; ... } I suggest changing the loop condition to something like l < lbuf + length. Thanks, //richard --- fs/jffs2/xattr.c | 2 ++ 1 file changed, 2 insertions(+)