From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-fsdevel-owner@vger.kernel.org>
Received: from mail-it0-f44.google.com ([209.85.214.44]:32816 "EHLO
        mail-it0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1752397AbcJZChX (ORCPT
        <rfc822;linux-fsdevel@vger.kernel.org>);
        Tue, 25 Oct 2016 22:37:23 -0400
Received: by mail-it0-f44.google.com with SMTP id e187so937582itc.0
        for <linux-fsdevel@vger.kernel.org>; Tue, 25 Oct 2016 19:37:23 -0700 (PDT)
Date: Tue, 25 Oct 2016 21:37:21 -0500
From: Seth Forshee <seth.forshee@canonical.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: linux-fsdevel@vger.kernel.org
Subject: Re: [bug report] fuse: Add posix ACL support
Message-ID: <20161026023721.GA109664@ubuntu-hedt>
References: <20161025213103.GA30419@elgon.mountain>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20161025213103.GA30419@elgon.mountain>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On Wed, Oct 26, 2016 at 12:31:03AM +0300, Dan Carpenter wrote:
> Hello Seth Forshee,
> 
> The patch 60bcc88ad185: "fuse: Add posix ACL support" from Aug 29,
> 2016, leads to the following static checker warning:
> 
> 	fs/fuse/acl.c:39 fuse_get_acl()
> 	warn: we tested 'fc->no_getxattr' before and it was 'false'
> 
> fs/fuse/acl.c
>     14  struct posix_acl *fuse_get_acl(struct inode *inode, int type)
>     15  {
>     16          struct fuse_conn *fc = get_fuse_conn(inode);
>     17          int size;
>     18          const char *name;
>     19          void *value = NULL;
>     20          struct posix_acl *acl;
>     21  
>     22          if (!fc->posix_acl || fc->no_getxattr)
> 
> If "fc->no_getxattr" is set then we return NULL.
> 
>     23                  return NULL;
>     24  
>     25          if (type == ACL_TYPE_ACCESS)
>     26                  name = XATTR_NAME_POSIX_ACL_ACCESS;
>     27          else if (type == ACL_TYPE_DEFAULT)
>     28                  name = XATTR_NAME_POSIX_ACL_DEFAULT;
>     29          else
>     30                  return ERR_PTR(-EOPNOTSUPP);
>     31  
>     32          value = kmalloc(PAGE_SIZE, GFP_KERNEL);
>     33          if (!value)
>     34                  return ERR_PTR(-ENOMEM);
>     35          size = fuse_getxattr(inode, name, value, PAGE_SIZE);
>     36          if (size > 0)
>     37                  acl = posix_acl_from_xattr(&init_user_ns, value, size);
>     38          else if ((size == 0) || (size == -ENODATA) ||
>     39                   (size == -EOPNOTSUPP && fc->no_getxattr))
>                                                  ^^^^^^^^^^^^^^^
> So we can't actually reach here.

This analysis is incorrect. fc->no_getxattr may be set by the
fuse_getxattr() call above.

Thanks,
Seth