From: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
To: Sheng Yong <shengyong1@huawei.com>, <dedekind1@gmail.com>,
<richard.weinberger@gmail.com>, <andreas.gruenbacher@gmail.com>
Cc: <linux-mtd@lists.infradead.org>, <linux-fsdevel@vger.kernel.org>
Subject: Re: [RFC PATCH v3 3/5] UBIFS: ACL: handle ACL through xattr
Date: Fri, 11 Sep 2015 14:25:25 +0800 [thread overview]
Message-ID: <55F273D5.5080905@cn.fujitsu.com> (raw)
In-Reply-To: <55F27240.2050500@huawei.com>
On 09/11/2015 02:18 PM, Sheng Yong wrote:
> Hi, Dongsheng
>
> On 9/11/2015 1:01 PM, Dongsheng Yang wrote:
>> On 09/11/2015 05:09 PM, Sheng Yong wrote:
> [...]
>>
>> Why move it? If you just want to use them before the definitions,
>> Just declare them before using.
>
> OK.
>>> int ubifs_do_setxattr(struct inode *host, const char *name,
>>> const void *value, size_t size, int flags)
>>> {
>>> @@ -348,7 +462,19 @@ int ubifs_do_setxattr(struct inode *host, const char *name,
>>> goto out_free;
>>> }
>>>
>>> +#ifdef CONFIG_UBIFS_FS_POSIX_ACL
>>> + if (size == 0) {
>>> + ubifs_assert(inode->i_nlink == 1);
>>> + clear_nlink(inode);
>>> + err = remove_xattr(c, host, inode, &nm);
>>> + if (err)
>>> + set_nlink(inode, 1);
>>> + iput(inode);
>>> + goto out_free;
>>> + }
>>> +#endif
>>
>> Is there a testcase for it?
>
> I test `setfacl -b/-k'. I don't know how setfacl is implemented. But for ACL_TYPE_ACCESS,
> ubifs_setxattr() is called with size = 0 and value = NULL; while for ACL_TYPE_DEFAULT,
> ubifs_removexattr() is called.
I mean, if we don't add the code above, what kind of problem
we would meet?
>
>>> err = change_xattr(c, host, inode, value, size);
>>> +
>>> iput(inode);
>>>
>>> out_free:
>>> @@ -359,6 +485,9 @@ out_free:
>>> int ubifs_setxattr(struct dentry *dentry, const char *name,
>>> const void *value, size_t size, int flags)
>>> {
>>> +#ifdef CONFIG_UBIFS_FS_POSIX_ACL
>>> + const struct xattr_handler *handler;
>>> +#endif
>>> struct qstr nm = QSTR_INIT(name, strlen(name));
>>> int type;
>>>
>>> @@ -369,6 +498,16 @@ int ubifs_setxattr(struct dentry *dentry, const char *name,
>>> if (type < 0)
>>> return type;
>>>
>>> +#ifdef CONFIG_UBIFS_FS_POSIX_ACL
>>> + if (type == POSIX_ACL_DEFAULT || type == POSIX_ACL_ACCESS) {
>>> + if (type == POSIX_ACL_DEFAULT)
>>> + handler = &posix_acl_default_xattr_handler;
>>> + if (type == POSIX_ACL_ACCESS)
>>> + handler = &posix_acl_access_xattr_handler;
>>> + return handler->set(dentry, name, value, size, flags,
>>> + handler->flags);
>>> + }
>>> +#endif
>>
>> What about setting sb->s_xattr and calling generic_setxattr() here?
>
> I have no idea if we should do this :(
> If we do, I think, we should call generic functions for all xattr.
No, only for POSIX_ACL_DEFAULT|POSIX_ACL_ACCESS currently. Then
Something like that:
if (type == POSIX_ACL_DEFAULT || type == POSIX_ACL_ACCESS)
return generic_setxattr(dentry, name, value, size, flags);
Yang
>
> thanks,
> Sheng
>
>>> return ubifs_do_setxattr(d_inode(dentry), name, value, size, flags);
>>> }
>>>
>>> @@ -428,6 +567,9 @@ out_unlock:
>>> ssize_t ubifs_getxattr(struct dentry *dentry, const char *name,
>>> void *value, size_t size)
>>> {
>>> +#ifdef CONFIG_UBIFS_FS_POSIX_ACL
>>> + const struct xattr_handler *handler;
>>> +#endif
>>> struct qstr nm = QSTR_INIT(name, strlen(name));
>>> int type;
>>>
>>> @@ -438,6 +580,16 @@ ssize_t ubifs_getxattr(struct dentry *dentry, const char *name,
>>> if (type < 0)
>>> return type;
>>>
>>> +#ifdef CONFIG_UBIFS_FS_POSIX_ACL
>>> + if (type == POSIX_ACL_DEFAULT || type == POSIX_ACL_ACCESS) {
>>> + if (type == POSIX_ACL_DEFAULT)
>>> + handler = &posix_acl_default_xattr_handler;
>>> + if (type == POSIX_ACL_ACCESS)
>>> + handler = &posix_acl_access_xattr_handler;
>>> + return handler->get(dentry, name, value, size,
>>> + handler->flags);
>>> + }
>>> +#endif
>>
>> Ditto
>>
>> Thanx
>> Yang
>>> return ubifs_do_getxattr(d_inode(dentry), name, value, size);
>>> }
>>>
>>> @@ -505,95 +657,6 @@ ssize_t ubifs_listxattr(struct dentry *dentry, char *buffer, size_t size)
>>> return written;
>>> }
>>>
>>> -static int remove_xattr(struct ubifs_info *c, struct inode *host,
>>> - struct inode *inode, const struct qstr *nm)
>>> -{
>>> - int err;
>>> - struct ubifs_inode *host_ui = ubifs_inode(host);
>>> - struct ubifs_inode *ui = ubifs_inode(inode);
>>> - struct ubifs_budget_req req = { .dirtied_ino = 2, .mod_dent = 1,
>>> - .dirtied_ino_d = ALIGN(host_ui->data_len, 8) };
>>> -
>>> - ubifs_assert(ui->data_len == inode->i_size);
>>> -
>>> - err = ubifs_budget_space(c, &req);
>>> - if (err)
>>> - return err;
>>> -
>>> - mutex_lock(&host_ui->ui_mutex);
>>> - host->i_ctime = ubifs_current_time(host);
>>> - host_ui->xattr_cnt -= 1;
>>> - host_ui->xattr_size -= CALC_DENT_SIZE(nm->len);
>>> - host_ui->xattr_size -= CALC_XATTR_BYTES(ui->data_len);
>>> - host_ui->xattr_names -= nm->len;
>>> -
>>> - err = ubifs_jnl_delete_xattr(c, host, inode, nm);
>>> - if (err)
>>> - goto out_cancel;
>>> - mutex_unlock(&host_ui->ui_mutex);
>>> -
>>> - ubifs_release_budget(c, &req);
>>> - return 0;
>>> -
>>> -out_cancel:
>>> - host_ui->xattr_cnt += 1;
>>> - host_ui->xattr_size += CALC_DENT_SIZE(nm->len);
>>> - host_ui->xattr_size += CALC_XATTR_BYTES(ui->data_len);
>>> - mutex_unlock(&host_ui->ui_mutex);
>>> - ubifs_release_budget(c, &req);
>>> - make_bad_inode(inode);
>>> - return err;
>>> -}
>>> -
>>> -int ubifs_removexattr(struct dentry *dentry, const char *name)
>>> -{
>>> - struct inode *inode, *host = d_inode(dentry);
>>> - struct ubifs_info *c = host->i_sb->s_fs_info;
>>> - struct qstr nm = QSTR_INIT(name, strlen(name));
>>> - struct ubifs_dent_node *xent;
>>> - union ubifs_key key;
>>> - int err;
>>> -
>>> - dbg_gen("xattr '%s', ino %lu ('%pd')", name,
>>> - host->i_ino, dentry);
>>> - ubifs_assert(mutex_is_locked(&host->i_mutex));
>>> -
>>> - err = check_namespace(&nm);
>>> - if (err < 0)
>>> - return err;
>>> -
>>> - xent = kmalloc(UBIFS_MAX_XENT_NODE_SZ, GFP_NOFS);
>>> - if (!xent)
>>> - return -ENOMEM;
>>> -
>>> - xent_key_init(c, &key, host->i_ino, &nm);
>>> - err = ubifs_tnc_lookup_nm(c, &key, xent, &nm);
>>> - if (err) {
>>> - if (err == -ENOENT)
>>> - err = -ENODATA;
>>> - goto out_free;
>>> - }
>>> -
>>> - inode = iget_xattr(c, le64_to_cpu(xent->inum));
>>> - if (IS_ERR(inode)) {
>>> - err = PTR_ERR(inode);
>>> - goto out_free;
>>> - }
>>> -
>>> - ubifs_assert(inode->i_nlink == 1);
>>> - clear_nlink(inode);
>>> - err = remove_xattr(c, host, inode, &nm);
>>> - if (err)
>>> - set_nlink(inode, 1);
>>> -
>>> - /* If @i_nlink is 0, 'iput()' will delete the inode */
>>> - iput(inode);
>>> -
>>> -out_free:
>>> - kfree(xent);
>>> - return err;
>>> -}
>>> -
>>> static size_t security_listxattr(struct dentry *d, char *list, size_t list_size,
>>> const char *name, size_t name_len, int flags)
>>> {
>>>
>>
>>
>> .
>>
>
> .
>
next prev parent reply other threads:[~2015-09-11 6:31 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-11 9:09 [RFC PATCH v3 0/5] UBIFS: add ACL support Sheng Yong
2015-09-11 9:09 ` [RFC PATCH v3 1/5] UBIFS: ACL: introduce init/set/get functions for ACL Sheng Yong
2015-09-11 5:01 ` Dongsheng Yang
2015-09-11 6:13 ` Sheng Yong
2015-09-11 6:21 ` Dongsheng Yang
2015-09-11 20:05 ` Andreas Grünbacher
2015-09-11 9:09 ` [RFC PATCH v3 2/5] UBIFS: ACL: set ACL interfaces in inode_operations Sheng Yong
2015-09-11 9:09 ` [RFC PATCH v3 3/5] UBIFS: ACL: handle ACL through xattr Sheng Yong
2015-09-11 5:01 ` Dongsheng Yang
2015-09-11 6:18 ` Sheng Yong
2015-09-11 6:25 ` Dongsheng Yang [this message]
2015-09-12 1:15 ` Sheng Yong
2015-09-14 0:39 ` Dongsheng Yang
2015-09-11 9:09 ` [RFC PATCH v3 4/5] UBIFS: ACL: introduce ACL mount options Sheng Yong
2015-09-11 5:03 ` Dongsheng Yang
2015-09-11 8:25 ` Sheng Yong
2015-09-11 8:25 ` Dongsheng Yang
2015-09-11 9:09 ` [RFC PATCH v3 5/5] UBIFS: ACL: add ACL config option Sheng Yong
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=55F273D5.5080905@cn.fujitsu.com \
--to=yangds.fnst@cn.fujitsu.com \
--cc=andreas.gruenbacher@gmail.com \
--cc=dedekind1@gmail.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=richard.weinberger@gmail.com \
--cc=shengyong1@huawei.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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).