[lustre-devel] [PATCH] staging/lustre/ptlrpc: Removes potential null dereference

[Lidza Louina <lidza.louina@oracle.com>]

On 05/13/2016 12:43 PM, Drokin, Oleg wrote:
> On May 12, 2016, at 6:33 PM, Lidza Louina wrote:
>
>> The lustre_msg_buf method could return NULL. Subsequent code didn't
>> check if it's null before using it. This patch adds two checks.
> Thank you for the patch, it looks good code-wise, but there are some style issues.
>
>>      
>> Signed-off-by: Lidza Louina <Lidza.Louina@oracle.com>
>>
>> diff --git a/drivers/staging/lustre/lustre/ptlrpc/sec.c b/drivers/staging/lustre/lustre/ptlrpc/sec.c
>> index 187fd1d..e6fedc3 100644
>> --- a/drivers/staging/lustre/lustre/ptlrpc/sec.c
>> +++ b/drivers/staging/lustre/lustre/ptlrpc/sec.c
>> @@ -2195,6 +2195,8 @@ int sptlrpc_pack_user_desc(struct lustre_msg *msg, int offset)
>>   	struct ptlrpc_user_desc *pud;
>>   
>>   	pud = lustre_msg_buf(msg, offset, 0);
>> +	if (!pud)
>> +		return -EINVAL;
>>   
>>   	pud->pud_uid = from_kuid(&init_user_ns, current_uid());
>>   	pud->pud_gid = from_kgid(&init_user_ns, current_gid());
>> diff --git a/drivers/staging/lustre/lustre/ptlrpc/sec_plain.c b/drivers/staging/lustre/lustre/ptlrpc/sec_plain.c
>> index 37c9f4c..7736aa9 100644
>> --- a/drivers/staging/lustre/lustre/ptlrpc/sec_plain.c
>> +++ b/drivers/staging/lustre/lustre/ptlrpc/sec_plain.c
>> @@ -542,6 +542,7 @@ int plain_alloc_reqbuf(struct ptlrpc_sec *sec,
>>   {
>>   	__u32 buflens[PLAIN_PACK_SEGMENTS] = { 0, };
>>   	int alloc_len;
>> +	int desc;
>>   
>>   	buflens[PLAIN_PACK_HDR_OFF] = sizeof(struct plain_header);
>>   	buflens[PLAIN_PACK_MSG_OFF] = msgsize;
>> @@ -575,7 +576,10 @@ int plain_alloc_reqbuf(struct ptlrpc_sec *sec,
>>   	req->rq_reqmsg = lustre_msg_buf(req->rq_reqbuf, PLAIN_PACK_MSG_OFF, 0);
>>   
>>   	if (req->rq_pack_udesc)
>> -		sptlrpc_pack_user_desc(req->rq_reqbuf, PLAIN_PACK_USER_OFF);
>> +		desc = sptlrpc_pack_user_desc(req->rq_reqbuf, PLAIN_PACK_USER_OFF);
> This makes the line longer than 80 chars.
> Also why not declare desc right here as opposed to the start of the function like you did?
>
>> +		if (!desc){
>> +			return desc;
>> +		}
> We don't really need these curvy braces here.
>
> Thanks!
>
>
Thanks for the feedback. I'm sending a new patch in now.

Lidza