Re: [PATCH v2] dmabuf: use spinlock to access dmabuf->name
Thanks Mike for the inputs. On 6/22/2020 5:10 PM, Ruhl, Michael J wrote: >> -Original Message- >> From: charante=codeaurora@mg.codeaurora.org >> On Behalf Of Charan Teja >> Kalla >> Sent: Monday, June 22, 2020 5:26 AM >> To: Ruhl, Michael J ; Sumit Semwal >> ; david.lai...@aculab.com; open list:DMA >> BUFFER SHARING FRAMEWORK ; DRI mailing >> list >> Cc: Linaro MM SIG ; LKML > ker...@vger.kernel.org> >> Subject: Re: [PATCH v2] dmabuf: use spinlock to access dmabuf->name >> >> Hello Mike, >> >> On 6/19/2020 7:11 PM, Ruhl, Michael J wrote: >>>> -Original Message- >>>> From: charante=codeaurora@mg.codeaurora.org >>>> On Behalf Of Charan >> Teja >>>> Kalla >>>> Sent: Friday, June 19, 2020 7:57 AM >>>> To: Sumit Semwal ; Ruhl, Michael J >>>> ; david.lai...@aculab.com; open list:DMA >>>> BUFFER SHARING FRAMEWORK ; DRI >> mailing >>>> list >>>> Cc: Linaro MM SIG ; LKML >>> ker...@vger.kernel.org> >>>> Subject: [PATCH v2] dmabuf: use spinlock to access dmabuf->name >>>> >>>> There exists a sleep-while-atomic bug while accessing the dmabuf->name >>>> under mutex in the dmabuffs_dname(). This is caused from the SELinux >>>> permissions checks on a process where it tries to validate the inherited >>>> files from fork() by traversing them through iterate_fd() (which >>>> traverse files under spin_lock) and call >>>> match_file(security/selinux/hooks.c) where the permission checks >> happen. >>>> This audit information is logged using dump_common_audit_data() where >> it >>>> calls d_path() to get the file path name. If the file check happen on >>>> the dmabuf's fd, then it ends up in ->dmabuffs_dname() and use mutex to >>>> access dmabuf->name. The flow will be like below: >>>> flush_unauthorized_files() >>>> iterate_fd() >>>>spin_lock() --> Start of the atomic section. >>>> match_file() >>>>file_has_perm() >>>> avc_has_perm() >>>>avc_audit() >>>> slow_avc_audit() >>>>common_lsm_audit() >>>> dump_common_audit_data() >>>>audit_log_d_path() >>>> d_path() >>>>dmabuffs_dname() >>>> mutex_lock()--> Sleep while atomic. >>>> >>>> Call trace captured (on 4.19 kernels) is below: >>>> ___might_sleep+0x204/0x208 >>>> __might_sleep+0x50/0x88 >>>> __mutex_lock_common+0x5c/0x1068 >>>> __mutex_lock_common+0x5c/0x1068 >>>> mutex_lock_nested+0x40/0x50 >>>> dmabuffs_dname+0xa0/0x170 >>>> d_path+0x84/0x290 >>>> audit_log_d_path+0x74/0x130 >>>> common_lsm_audit+0x334/0x6e8 >>>> slow_avc_audit+0xb8/0xf8 >>>> avc_has_perm+0x154/0x218 >>>> file_has_perm+0x70/0x180 >>>> match_file+0x60/0x78 >>>> iterate_fd+0x128/0x168 >>>> selinux_bprm_committing_creds+0x178/0x248 >>>> security_bprm_committing_creds+0x30/0x48 >>>> install_exec_creds+0x1c/0x68 >>>> load_elf_binary+0x3a4/0x14e0 >>>> search_binary_handler+0xb0/0x1e0 >>>> >>>> So, use spinlock to access dmabuf->name to avoid sleep-while-atomic. >>>> >>>> Cc: [5.3+] >>>> Signed-off-by: Charan Teja Reddy >>>> --- >>>> >>>> Changes in V2: Addressed review comments from Ruhl, Michael J >>>> >>>> Changes in V1: https://lore.kernel.org/patchwork/patch/1255055/ >>>> >>>> drivers/dma-buf/dma-buf.c | 11 +++ >>>> include/linux/dma-buf.h | 1 + >>>> 2 files changed, 8 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c >>>> index 01ce125..d81d298 100644 >>>> --- a/drivers/dma-buf/dma-buf.c >>>> +++ b/drivers/dma-buf/dma-buf.c >>>> @@ -45,10 +45,10 @@ static char *dmabuffs_dname(struct dentry >> *dentry, >>>> char *buffer, int buflen) >>>>size_t ret = 0; >>>> >>>>dmabuf = dentry->d_fsdata; >>>> - dma_resv_lock(dmabuf->resv, NULL); >>>> + spin_lock(>name_lock); >>>>if (dmabuf-&g
RE: [PATCH v2] dmabuf: use spinlock to access dmabuf->name
>-Original Message- >From: charante=codeaurora@mg.codeaurora.org > On Behalf Of Charan Teja >Kalla >Sent: Tuesday, June 23, 2020 7:34 AM >To: Ruhl, Michael J ; Sumit Semwal >; david.lai...@aculab.com; open list:DMA >BUFFER SHARING FRAMEWORK ; DRI mailing >list >Cc: Linaro MM SIG ; LKML ker...@vger.kernel.org> >Subject: Re: [PATCH v2] dmabuf: use spinlock to access dmabuf->name > >Thanks Mike for the inputs. > >On 6/22/2020 5:10 PM, Ruhl, Michael J wrote: >>> -Original Message- >>> From: charante=codeaurora@mg.codeaurora.org >>> On Behalf Of Charan >Teja >>> Kalla >>> Sent: Monday, June 22, 2020 5:26 AM >>> To: Ruhl, Michael J ; Sumit Semwal >>> ; david.lai...@aculab.com; open list:DMA >>> BUFFER SHARING FRAMEWORK ; DRI >mailing >>> list >>> Cc: Linaro MM SIG ; LKML >> ker...@vger.kernel.org> >>> Subject: Re: [PATCH v2] dmabuf: use spinlock to access dmabuf->name >>> >>> Hello Mike, >>> >>> On 6/19/2020 7:11 PM, Ruhl, Michael J wrote: >>>>> -Original Message- >>>>> From: charante=codeaurora@mg.codeaurora.org >>>>> On Behalf Of Charan >>> Teja >>>>> Kalla >>>>> Sent: Friday, June 19, 2020 7:57 AM >>>>> To: Sumit Semwal ; Ruhl, Michael J >>>>> ; david.lai...@aculab.com; open >list:DMA >>>>> BUFFER SHARING FRAMEWORK ; DRI >>> mailing >>>>> list >>>>> Cc: Linaro MM SIG ; LKML >>>> ker...@vger.kernel.org> >>>>> Subject: [PATCH v2] dmabuf: use spinlock to access dmabuf->name >>>>> >>>>> There exists a sleep-while-atomic bug while accessing the dmabuf- >>name >>>>> under mutex in the dmabuffs_dname(). This is caused from the SELinux >>>>> permissions checks on a process where it tries to validate the inherited >>>>> files from fork() by traversing them through iterate_fd() (which >>>>> traverse files under spin_lock) and call >>>>> match_file(security/selinux/hooks.c) where the permission checks >>> happen. >>>>> This audit information is logged using dump_common_audit_data() >where >>> it >>>>> calls d_path() to get the file path name. If the file check happen on >>>>> the dmabuf's fd, then it ends up in ->dmabuffs_dname() and use mutex >to >>>>> access dmabuf->name. The flow will be like below: >>>>> flush_unauthorized_files() >>>>> iterate_fd() >>>>>spin_lock() --> Start of the atomic section. >>>>> match_file() >>>>>file_has_perm() >>>>> avc_has_perm() >>>>>avc_audit() >>>>> slow_avc_audit() >>>>> common_lsm_audit() >>>>> dump_common_audit_data() >>>>> audit_log_d_path() >>>>> d_path() >>>>>dmabuffs_dname() >>>>> mutex_lock()--> Sleep while atomic. >>>>> >>>>> Call trace captured (on 4.19 kernels) is below: >>>>> ___might_sleep+0x204/0x208 >>>>> __might_sleep+0x50/0x88 >>>>> __mutex_lock_common+0x5c/0x1068 >>>>> __mutex_lock_common+0x5c/0x1068 >>>>> mutex_lock_nested+0x40/0x50 >>>>> dmabuffs_dname+0xa0/0x170 >>>>> d_path+0x84/0x290 >>>>> audit_log_d_path+0x74/0x130 >>>>> common_lsm_audit+0x334/0x6e8 >>>>> slow_avc_audit+0xb8/0xf8 >>>>> avc_has_perm+0x154/0x218 >>>>> file_has_perm+0x70/0x180 >>>>> match_file+0x60/0x78 >>>>> iterate_fd+0x128/0x168 >>>>> selinux_bprm_committing_creds+0x178/0x248 >>>>> security_bprm_committing_creds+0x30/0x48 >>>>> install_exec_creds+0x1c/0x68 >>>>> load_elf_binary+0x3a4/0x14e0 >>>>> search_binary_handler+0xb0/0x1e0 >>>>> >>>>> So, use spinlock to access dmabuf->name to avoid sleep-while-atomic. >>>>> >>>>> Cc: [5.3+] >>>>> Signed-off-by: Charan Teja Reddy >>>>> --- >>>>> >>>>> Changes in V2: Addressed review comments from Ruhl, Michael J >>>>> >>>>> Changes in V1: https://lore.kernel.org/patchwork/p
Re: [PATCH v2] dmabuf: use spinlock to access dmabuf->name
Hello Mike,
On 6/19/2020 7:11 PM, Ruhl, Michael J wrote:
>> -Original Message-
>> From: charante=codeaurora@mg.codeaurora.org
>> On Behalf Of Charan Teja
>> Kalla
>> Sent: Friday, June 19, 2020 7:57 AM
>> To: Sumit Semwal ; Ruhl, Michael J
>> ; david.lai...@aculab.com; open list:DMA
>> BUFFER SHARING FRAMEWORK ; DRI mailing
>> list
>> Cc: Linaro MM SIG ; LKML > ker...@vger.kernel.org>
>> Subject: [PATCH v2] dmabuf: use spinlock to access dmabuf->name
>>
>> There exists a sleep-while-atomic bug while accessing the dmabuf->name
>> under mutex in the dmabuffs_dname(). This is caused from the SELinux
>> permissions checks on a process where it tries to validate the inherited
>> files from fork() by traversing them through iterate_fd() (which
>> traverse files under spin_lock) and call
>> match_file(security/selinux/hooks.c) where the permission checks happen.
>> This audit information is logged using dump_common_audit_data() where it
>> calls d_path() to get the file path name. If the file check happen on
>> the dmabuf's fd, then it ends up in ->dmabuffs_dname() and use mutex to
>> access dmabuf->name. The flow will be like below:
>> flush_unauthorized_files()
>> iterate_fd()
>>spin_lock() --> Start of the atomic section.
>> match_file()
>>file_has_perm()
>> avc_has_perm()
>>avc_audit()
>> slow_avc_audit()
>> common_lsm_audit()
>>dump_common_audit_data()
>> audit_log_d_path()
>>d_path()
>>dmabuffs_dname()
>> mutex_lock()--> Sleep while atomic.
>>
>> Call trace captured (on 4.19 kernels) is below:
>> ___might_sleep+0x204/0x208
>> __might_sleep+0x50/0x88
>> __mutex_lock_common+0x5c/0x1068
>> __mutex_lock_common+0x5c/0x1068
>> mutex_lock_nested+0x40/0x50
>> dmabuffs_dname+0xa0/0x170
>> d_path+0x84/0x290
>> audit_log_d_path+0x74/0x130
>> common_lsm_audit+0x334/0x6e8
>> slow_avc_audit+0xb8/0xf8
>> avc_has_perm+0x154/0x218
>> file_has_perm+0x70/0x180
>> match_file+0x60/0x78
>> iterate_fd+0x128/0x168
>> selinux_bprm_committing_creds+0x178/0x248
>> security_bprm_committing_creds+0x30/0x48
>> install_exec_creds+0x1c/0x68
>> load_elf_binary+0x3a4/0x14e0
>> search_binary_handler+0xb0/0x1e0
>>
>> So, use spinlock to access dmabuf->name to avoid sleep-while-atomic.
>>
>> Cc: [5.3+]
>> Signed-off-by: Charan Teja Reddy
>> ---
>>
>> Changes in V2: Addressed review comments from Ruhl, Michael J
>>
>> Changes in V1: https://lore.kernel.org/patchwork/patch/1255055/
>>
>> drivers/dma-buf/dma-buf.c | 11 +++
>> include/linux/dma-buf.h | 1 +
>> 2 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>> index 01ce125..d81d298 100644
>> --- a/drivers/dma-buf/dma-buf.c
>> +++ b/drivers/dma-buf/dma-buf.c
>> @@ -45,10 +45,10 @@ static char *dmabuffs_dname(struct dentry *dentry,
>> char *buffer, int buflen)
>> size_t ret = 0;
>>
>> dmabuf = dentry->d_fsdata;
>> -dma_resv_lock(dmabuf->resv, NULL);
>> +spin_lock(>name_lock);
>> if (dmabuf->name)
>> ret = strlcpy(name, dmabuf->name, DMA_BUF_NAME_LEN);
>> -dma_resv_unlock(dmabuf->resv);
>> +spin_unlock(>name_lock);
>>
>> return dynamic_dname(dentry, buffer, buflen, "/%s:%s",
>> dentry->d_name.name, ret > 0 ? name : "");
>> @@ -341,8 +341,10 @@ static long dma_buf_set_name(struct dma_buf
>> *dmabuf, const char __user *buf)
>> kfree(name);
>> goto out_unlock;
>> }
>> +spin_lock(>name_lock);
>> kfree(dmabuf->name);
>> dmabuf->name = name;
>> +spin_unlock(>name_lock);
>
> While this code path is ok, I would have separated the protection of the
> attachment list and the name manipulation.
>
> dma_resv_lock(resv)
> if (!list_empty(attachment)
> ret = -EBUSY
> dma_resv_unlock(resv)
>
> if (ret) {
> kfree(name)
> return ret;
> }
Is it that the name should be visible before importer attaches to the
dmabuf,(using dma_buf_attach()), thus _buf_set_name() is under the
_resv_lock() as well?
>
> spinlock(nam_lock)
> ...
>
> Nesting locks that don't need to be nested always makes me nervous
> for future use that misses the lock/unlock pattern.
>
> However, this looks reasonable.
>
> With this current code, or if you update to the above pattern:
>
> Reviewed-by: Michael J. Ruhl
Thanks for the ACK.
>
> Mike
>
>
>> out_unlock:
>> dma_resv_unlock(dmabuf->resv);
>> @@ -405,10 +407,10 @@ static void dma_buf_show_fdinfo(struct seq_file
>> *m, struct file *file)
>> /* Don't count the temporary reference taken inside procfs seq_show
>> */
>> seq_printf(m, "count:\t%ld\n", file_count(dmabuf->file) - 1);
>> seq_printf(m, "exp_name:\t%s\n", dmabuf->exp_name);
>> -dma_resv_lock(dmabuf->resv, NULL);
>> +spin_lock(>name_lock);
>> if (dmabuf->name)
>> seq_printf(m,
RE: [PATCH v2] dmabuf: use spinlock to access dmabuf->name
>-Original Message- >From: charante=codeaurora@mg.codeaurora.org > On Behalf Of Charan Teja >Kalla >Sent: Monday, June 22, 2020 5:26 AM >To: Ruhl, Michael J ; Sumit Semwal >; david.lai...@aculab.com; open list:DMA >BUFFER SHARING FRAMEWORK ; DRI mailing >list >Cc: Linaro MM SIG ; LKML ker...@vger.kernel.org> >Subject: Re: [PATCH v2] dmabuf: use spinlock to access dmabuf->name > >Hello Mike, > >On 6/19/2020 7:11 PM, Ruhl, Michael J wrote: >>> -Original Message- >>> From: charante=codeaurora@mg.codeaurora.org >>> On Behalf Of Charan >Teja >>> Kalla >>> Sent: Friday, June 19, 2020 7:57 AM >>> To: Sumit Semwal ; Ruhl, Michael J >>> ; david.lai...@aculab.com; open list:DMA >>> BUFFER SHARING FRAMEWORK ; DRI >mailing >>> list >>> Cc: Linaro MM SIG ; LKML >> ker...@vger.kernel.org> >>> Subject: [PATCH v2] dmabuf: use spinlock to access dmabuf->name >>> >>> There exists a sleep-while-atomic bug while accessing the dmabuf->name >>> under mutex in the dmabuffs_dname(). This is caused from the SELinux >>> permissions checks on a process where it tries to validate the inherited >>> files from fork() by traversing them through iterate_fd() (which >>> traverse files under spin_lock) and call >>> match_file(security/selinux/hooks.c) where the permission checks >happen. >>> This audit information is logged using dump_common_audit_data() where >it >>> calls d_path() to get the file path name. If the file check happen on >>> the dmabuf's fd, then it ends up in ->dmabuffs_dname() and use mutex to >>> access dmabuf->name. The flow will be like below: >>> flush_unauthorized_files() >>> iterate_fd() >>>spin_lock() --> Start of the atomic section. >>> match_file() >>>file_has_perm() >>> avc_has_perm() >>>avc_audit() >>> slow_avc_audit() >>> common_lsm_audit() >>> dump_common_audit_data() >>> audit_log_d_path() >>> d_path() >>>dmabuffs_dname() >>> mutex_lock()--> Sleep while atomic. >>> >>> Call trace captured (on 4.19 kernels) is below: >>> ___might_sleep+0x204/0x208 >>> __might_sleep+0x50/0x88 >>> __mutex_lock_common+0x5c/0x1068 >>> __mutex_lock_common+0x5c/0x1068 >>> mutex_lock_nested+0x40/0x50 >>> dmabuffs_dname+0xa0/0x170 >>> d_path+0x84/0x290 >>> audit_log_d_path+0x74/0x130 >>> common_lsm_audit+0x334/0x6e8 >>> slow_avc_audit+0xb8/0xf8 >>> avc_has_perm+0x154/0x218 >>> file_has_perm+0x70/0x180 >>> match_file+0x60/0x78 >>> iterate_fd+0x128/0x168 >>> selinux_bprm_committing_creds+0x178/0x248 >>> security_bprm_committing_creds+0x30/0x48 >>> install_exec_creds+0x1c/0x68 >>> load_elf_binary+0x3a4/0x14e0 >>> search_binary_handler+0xb0/0x1e0 >>> >>> So, use spinlock to access dmabuf->name to avoid sleep-while-atomic. >>> >>> Cc: [5.3+] >>> Signed-off-by: Charan Teja Reddy >>> --- >>> >>> Changes in V2: Addressed review comments from Ruhl, Michael J >>> >>> Changes in V1: https://lore.kernel.org/patchwork/patch/1255055/ >>> >>> drivers/dma-buf/dma-buf.c | 11 +++ >>> include/linux/dma-buf.h | 1 + >>> 2 files changed, 8 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c >>> index 01ce125..d81d298 100644 >>> --- a/drivers/dma-buf/dma-buf.c >>> +++ b/drivers/dma-buf/dma-buf.c >>> @@ -45,10 +45,10 @@ static char *dmabuffs_dname(struct dentry >*dentry, >>> char *buffer, int buflen) >>> size_t ret = 0; >>> >>> dmabuf = dentry->d_fsdata; >>> - dma_resv_lock(dmabuf->resv, NULL); >>> + spin_lock(>name_lock); >>> if (dmabuf->name) >>> ret = strlcpy(name, dmabuf->name, DMA_BUF_NAME_LEN); >>> - dma_resv_unlock(dmabuf->resv); >>> + spin_unlock(>name_lock); >>> >>> return dynamic_dname(dentry, buffer, buflen, "/%s:%s", >>> dentry->d_name.name, ret > 0 ? name : ""); >>> @@ -341,8 +341,10 @@ static long dma_buf_set_name(struct dma_buf >>> *dmabuf, const char __user *buf) >>>
RE: [PATCH v2] dmabuf: use spinlock to access dmabuf->name
>-Original Message-
>From: charante=codeaurora@mg.codeaurora.org
> On Behalf Of Charan Teja
>Kalla
>Sent: Friday, June 19, 2020 7:57 AM
>To: Sumit Semwal ; Ruhl, Michael J
>; david.lai...@aculab.com; open list:DMA
>BUFFER SHARING FRAMEWORK ; DRI mailing
>list
>Cc: Linaro MM SIG ; LKML ker...@vger.kernel.org>
>Subject: [PATCH v2] dmabuf: use spinlock to access dmabuf->name
>
>There exists a sleep-while-atomic bug while accessing the dmabuf->name
>under mutex in the dmabuffs_dname(). This is caused from the SELinux
>permissions checks on a process where it tries to validate the inherited
>files from fork() by traversing them through iterate_fd() (which
>traverse files under spin_lock) and call
>match_file(security/selinux/hooks.c) where the permission checks happen.
>This audit information is logged using dump_common_audit_data() where it
>calls d_path() to get the file path name. If the file check happen on
>the dmabuf's fd, then it ends up in ->dmabuffs_dname() and use mutex to
>access dmabuf->name. The flow will be like below:
>flush_unauthorized_files()
> iterate_fd()
>spin_lock() --> Start of the atomic section.
> match_file()
>file_has_perm()
> avc_has_perm()
>avc_audit()
> slow_avc_audit()
> common_lsm_audit()
> dump_common_audit_data()
> audit_log_d_path()
> d_path()
>dmabuffs_dname()
> mutex_lock()--> Sleep while atomic.
>
>Call trace captured (on 4.19 kernels) is below:
>___might_sleep+0x204/0x208
>__might_sleep+0x50/0x88
>__mutex_lock_common+0x5c/0x1068
>__mutex_lock_common+0x5c/0x1068
>mutex_lock_nested+0x40/0x50
>dmabuffs_dname+0xa0/0x170
>d_path+0x84/0x290
>audit_log_d_path+0x74/0x130
>common_lsm_audit+0x334/0x6e8
>slow_avc_audit+0xb8/0xf8
>avc_has_perm+0x154/0x218
>file_has_perm+0x70/0x180
>match_file+0x60/0x78
>iterate_fd+0x128/0x168
>selinux_bprm_committing_creds+0x178/0x248
>security_bprm_committing_creds+0x30/0x48
>install_exec_creds+0x1c/0x68
>load_elf_binary+0x3a4/0x14e0
>search_binary_handler+0xb0/0x1e0
>
>So, use spinlock to access dmabuf->name to avoid sleep-while-atomic.
>
>Cc: [5.3+]
>Signed-off-by: Charan Teja Reddy
>---
>
>Changes in V2: Addressed review comments from Ruhl, Michael J
>
>Changes in V1: https://lore.kernel.org/patchwork/patch/1255055/
>
> drivers/dma-buf/dma-buf.c | 11 +++
> include/linux/dma-buf.h | 1 +
> 2 files changed, 8 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>index 01ce125..d81d298 100644
>--- a/drivers/dma-buf/dma-buf.c
>+++ b/drivers/dma-buf/dma-buf.c
>@@ -45,10 +45,10 @@ static char *dmabuffs_dname(struct dentry *dentry,
>char *buffer, int buflen)
> size_t ret = 0;
>
> dmabuf = dentry->d_fsdata;
>- dma_resv_lock(dmabuf->resv, NULL);
>+ spin_lock(>name_lock);
> if (dmabuf->name)
> ret = strlcpy(name, dmabuf->name, DMA_BUF_NAME_LEN);
>- dma_resv_unlock(dmabuf->resv);
>+ spin_unlock(>name_lock);
>
> return dynamic_dname(dentry, buffer, buflen, "/%s:%s",
>dentry->d_name.name, ret > 0 ? name : "");
>@@ -341,8 +341,10 @@ static long dma_buf_set_name(struct dma_buf
>*dmabuf, const char __user *buf)
> kfree(name);
> goto out_unlock;
> }
>+ spin_lock(>name_lock);
> kfree(dmabuf->name);
> dmabuf->name = name;
>+ spin_unlock(>name_lock);
While this code path is ok, I would have separated the protection of the
attachment list and the name manipulation.
dma_resv_lock(resv)
if (!list_empty(attachment)
ret = -EBUSY
dma_resv_unlock(resv)
if (ret) {
kfree(name)
return ret;
}
spinlock(nam_lock)
...
Nesting locks that don't need to be nested always makes me nervous
for future use that misses the lock/unlock pattern.
However, this looks reasonable.
With this current code, or if you update to the above pattern:
Reviewed-by: Michael J. Ruhl
Mike
> out_unlock:
> dma_resv_unlock(dmabuf->resv);
>@@ -405,10 +407,10 @@ static void dma_buf_show_fdinfo(struct seq_file
>*m, struct file *file)
> /* Don't count the temporary reference taken inside procfs seq_show
>*/
> seq_printf(m, "count:\t%ld\n", file_count(dmabuf->file) - 1);
> seq_printf(m, "exp_name:\t%s\n", dmabuf->exp_name);
>- dma_resv_lock(dmabuf->resv, NULL);
>+ spin_lock(>name_lock);
> if (dmabuf->name)
> seq_printf(m, "name:\t%s\n", dmabuf->name);
>- dma_resv_unlock(dmabuf->resv);
>+ spin_unlock(>name_lock);
> }
>
> static const struct file_operations dma_buf_fops = {
>@@ -546,6 +548,7 @@ struct dma_buf *dma_buf_export(const struct
>dma_buf_export_info *exp_info)
> dmabuf->size = exp_info->size;
> dmabuf->exp_name = exp_info->exp_name;
> dmabuf->owner = exp_info->owner;
>+ spin_lock_init(>name_lock);
>
