* [lustre-devel] [PATCH] staging/lustre/ptlrpc: Removes potential null dereference
@ 2016-05-12 22:33 Lidza Louina
2016-05-13 16:43 ` Drokin, Oleg
0 siblings, 1 reply; 3+ messages in thread
From: Lidza Louina @ 2016-05-12 22:33 UTC (permalink / raw)
To: lustre-devel
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.
????
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);
+ if (!desc){
+ return desc;
+ }
?
? return 0;
?}
^ permalink raw reply related [flat|nested] 3+ messages in thread* [lustre-devel] [PATCH] staging/lustre/ptlrpc: Removes potential null dereference
2016-05-12 22:33 [lustre-devel] [PATCH] staging/lustre/ptlrpc: Removes potential null dereference Lidza Louina
@ 2016-05-13 16:43 ` Drokin, Oleg
2016-05-16 14:11 ` Lidza Louina
0 siblings, 1 reply; 3+ messages in thread
From: Drokin, Oleg @ 2016-05-13 16:43 UTC (permalink / raw)
To: lustre-devel
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!
^ permalink raw reply [flat|nested] 3+ messages in thread* [lustre-devel] [PATCH] staging/lustre/ptlrpc: Removes potential null dereference
2016-05-13 16:43 ` Drokin, Oleg
@ 2016-05-16 14:11 ` Lidza Louina
0 siblings, 0 replies; 3+ messages in thread
From: Lidza Louina @ 2016-05-16 14:11 UTC (permalink / raw)
To: lustre-devel
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-05-16 14:11 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-12 22:33 [lustre-devel] [PATCH] staging/lustre/ptlrpc: Removes potential null dereference Lidza Louina
2016-05-13 16:43 ` Drokin, Oleg
2016-05-16 14:11 ` Lidza Louina
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.