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