2018-05-02 03:06:55

by James Simmons

[permalink] [raw]
Subject: Re: [PATCH 10/10] staging: lustre: fix error deref in ll_splice_alias().


> d_splice_alias() can return an ERR_PTR().
> If it does while debugging is enabled, the following
> CDEBUG() will dereference that error and crash.
>
> So add appropriate checking, and provide a separate
> debug message for the error case.

Yeah!!! It fixed the issues. Thank you.

Reviewed-by: James Simmons <[email protected]>

> Reported-by: James Simmons <[email protected]>
> Fixes: e9d4f0b9f559 ("staging: lustre: llite: use d_splice_alias for directories.")
> Signed-off-by: NeilBrown <[email protected]>
> ---
> drivers/staging/lustre/lustre/llite/namei.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/llite/namei.c b/drivers/staging/lustre/lustre/llite/namei.c
> index 6c9ec462eb41..24a6873d86a2 100644
> --- a/drivers/staging/lustre/lustre/llite/namei.c
> +++ b/drivers/staging/lustre/lustre/llite/namei.c
> @@ -442,11 +442,15 @@ struct dentry *ll_splice_alias(struct inode *inode, struct dentry *de)
> } else {
> struct dentry *new = d_splice_alias(inode, de);
>
> + if (IS_ERR(new))
> + CDEBUG(D_DENTRY, "splice inode %p as %pd gives error %lu\n",
> + inode, de, PTR_ERR(new));
> if (new)
> de = new;
> }
> - CDEBUG(D_DENTRY, "Add dentry %p inode %p refc %d flags %#x\n",
> - de, d_inode(de), d_count(de), de->d_flags);
> + if (!IS_ERR(de))
> + CDEBUG(D_DENTRY, "Add dentry %p inode %p refc %d flags %#x\n",
> + de, d_inode(de), d_count(de), de->d_flags);
> return de;
> }
>
>
>
>


2018-05-04 00:36:35

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH 10/10] staging: lustre: fix error deref in ll_splice_alias().

On Wed, May 02 2018, James Simmons wrote:

>> d_splice_alias() can return an ERR_PTR().
>> If it does while debugging is enabled, the following
>> CDEBUG() will dereference that error and crash.
>>
>> So add appropriate checking, and provide a separate
>> debug message for the error case.
>
> Yeah!!! It fixed the issues. Thank you.

:-)
So I've made it "Reported-and-tested-by: James"

Thanks,
NeilBrown


>
> Reviewed-by: James Simmons <[email protected]>
>
>> Reported-by: James Simmons <[email protected]>
>> Fixes: e9d4f0b9f559 ("staging: lustre: llite: use d_splice_alias for directories.")
>> Signed-off-by: NeilBrown <[email protected]>
>> ---
>> drivers/staging/lustre/lustre/llite/namei.c | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/lustre/lustre/llite/namei.c b/drivers/staging/lustre/lustre/llite/namei.c
>> index 6c9ec462eb41..24a6873d86a2 100644
>> --- a/drivers/staging/lustre/lustre/llite/namei.c
>> +++ b/drivers/staging/lustre/lustre/llite/namei.c
>> @@ -442,11 +442,15 @@ struct dentry *ll_splice_alias(struct inode *inode, struct dentry *de)
>> } else {
>> struct dentry *new = d_splice_alias(inode, de);
>>
>> + if (IS_ERR(new))
>> + CDEBUG(D_DENTRY, "splice inode %p as %pd gives error %lu\n",
>> + inode, de, PTR_ERR(new));
>> if (new)
>> de = new;
>> }
>> - CDEBUG(D_DENTRY, "Add dentry %p inode %p refc %d flags %#x\n",
>> - de, d_inode(de), d_count(de), de->d_flags);
>> + if (!IS_ERR(de))
>> + CDEBUG(D_DENTRY, "Add dentry %p inode %p refc %d flags %#x\n",
>> + de, d_inode(de), d_count(de), de->d_flags);
>> return de;
>> }
>>
>>
>>
>>


Attachments:
signature.asc (847.00 B)