From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ua0-f181.google.com ([209.85.217.181]:34703 "EHLO mail-ua0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751170AbdBWV0q (ORCPT ); Thu, 23 Feb 2017 16:26:46 -0500 Received: by mail-ua0-f181.google.com with SMTP id 60so345498uah.1 for ; Thu, 23 Feb 2017 13:26:25 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <20170223203425.GJ9417@parsley.fieldses.org> References: <1487861534-14429-1-git-send-email-agruenba@redhat.com> <20170223203425.GJ9417@parsley.fieldses.org> From: Andreas Gruenbacher Date: Thu, 23 Feb 2017 22:26:24 +0100 Message-ID: Subject: Re: [PATCH] xattr: Additional maximum size check To: "J. Bruce Fields" Cc: Al Viro , Miklos Szeredi , linux-fsdevel Content-Type: text/plain; charset=UTF-8 Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Thu, Feb 23, 2017 at 9:34 PM, J. Bruce Fields wrote: > On Thu, Feb 23, 2017 at 03:52:14PM +0100, Andreas Gruenbacher wrote: >> When querying for the buffer size required, a filesystem's getxattr >> xattr handler or listxattr iop may return a value bigger than the >> maximum size limit. However, the VFS will never return oversize >> buffers, so cast such values to -E2BIG. >> >> Signed-off-by: Andreas Gruenbacher >> --- >> fs/xattr.c | 20 ++++++++++++-------- >> 1 file changed, 12 insertions(+), 8 deletions(-) >> >> diff --git a/fs/xattr.c b/fs/xattr.c >> index 7e3317c..c19a163 100644 >> --- a/fs/xattr.c >> +++ b/fs/xattr.c >> @@ -537,16 +537,18 @@ getxattr(struct dentry *d, const char __user *name, void __user *value, >> } >> >> error = vfs_getxattr(d, kname, kvalue, size); > > We have this just above: > > if (size) { > if (size > XATTR_SIZE_MAX) > size = XATTR_SIZE_MAX; > > So this test: > >> + if (error > XATTR_SIZE_MAX || >> + (error == -ERANGE && size >= XATTR_SIZE_MAX)) { >> + /* The file system tried to returned a value bigger >> + than XATTR_SIZE_MAX bytes. Not possible. */ >> + error = -E2BIG; >> + } > > is equivalent to: > > if (error > XATTR_SIZE_MAX || > (error = -ERANGE && size == XATTR_SIZE_MAX)) > > That's kind of a subtle case. True. > I guess the idea is that ERANGE means > "the xattr value doesn't fit into the buffer you gave me", and E2BIG > means "the xattr value couldn't fit in any buffer you could possible > give me". And if the filesystem isn't satisfied even with a > maximum-sized buffer then clearly we're in the second case. OK, got it. > Still, this is a little subtle. I don't see EF2BIG in my copy of > getxattr(2). It's not in any of my copies of getxattr(2) either yet, but it will be soon :) > Anyway, feel free to add > > Reviewed-by: J. Bruce Fields > > but I wouldn't say no to more documentation. Thanks, Andreas