[PATCH -next] audit: use struct_size() helper in kmalloc()

[PATCH -next] audit: use struct_size() helper in kmalloc()

[Xiu Jianfeng]

Make use of struct_size() helper instead of an open-coded calucation.

Link: https://github.com/KSPP/linux/issues/160
Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>
---
 kernel/audit.c       | 2 +-
 kernel/audit_tree.c  | 2 +-
 kernel/auditfilter.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index d4084751cfe6..f33028578c60 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1446,7 +1446,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 			if (err)
 				return err;
 		}
-		sig_data = kmalloc(sizeof(*sig_data) + len, GFP_KERNEL);
+		sig_data = kmalloc(struct_size(sig_data, ctx, len), GFP_KERNEL);
 		if (!sig_data) {
 			if (audit_sig_sid)
 				security_release_secctx(ctx, len);
diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
index 72324afcffef..e7315d487163 100644
--- a/kernel/audit_tree.c
+++ b/kernel/audit_tree.c
@@ -94,7 +94,7 @@ static struct audit_tree *alloc_tree(const char *s)
 {
 	struct audit_tree *tree;
 
-	tree = kmalloc(sizeof(struct audit_tree) + strlen(s) + 1, GFP_KERNEL);
+	tree = kmalloc(struct_size(tree, pathname, strlen(s) + 1), GFP_KERNEL);
 	if (tree) {
 		refcount_set(&tree->count, 1);
 		tree->goner = 0;
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 4173e771650c..19352820b274 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -637,7 +637,7 @@ static struct audit_rule_data *audit_krule_to_data(struct audit_krule *krule)
 	void *bufp;
 	int i;
 
-	data = kmalloc(sizeof(*data) + krule->buflen, GFP_KERNEL);
+	data = kmalloc(struct_size(data, buf, krule->buflen), GFP_KERNEL);
 	if (unlikely(!data))
 		return NULL;
 	memset(data, 0, sizeof(*data));
-- 
2.17.1

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit

Re: [PATCH -next] audit: use struct_size() helper in kmalloc()

[Kees Cook]

On Tue, Dec 14, 2021 at 07:48:54PM +0800, Xiu Jianfeng wrote:
> Make use of struct_size() helper instead of an open-coded calucation.
> 
> Link: https://github.com/KSPP/linux/issues/160
> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit

Re: [PATCH -next] audit: use struct_size() helper in kmalloc()

[Gustavo A. R. Silva]

On Tue, Dec 14, 2021 at 07:48:54PM +0800, Xiu Jianfeng wrote:
> Make use of struct_size() helper instead of an open-coded calucation.
> 
> Link: https://github.com/KSPP/linux/issues/160
> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>
> ---
>  kernel/audit.c       | 2 +-
>  kernel/audit_tree.c  | 2 +-
>  kernel/auditfilter.c | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/audit.c b/kernel/audit.c
> index d4084751cfe6..f33028578c60 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1446,7 +1446,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>  			if (err)
>  				return err;
>  		}
> -		sig_data = kmalloc(sizeof(*sig_data) + len, GFP_KERNEL);
> +		sig_data = kmalloc(struct_size(sig_data, ctx, len), GFP_KERNEL);
>  		if (!sig_data) {
>  			if (audit_sig_sid)
>  				security_release_secctx(ctx, len);
> diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
> index 72324afcffef..e7315d487163 100644
> --- a/kernel/audit_tree.c
> +++ b/kernel/audit_tree.c
> @@ -94,7 +94,7 @@ static struct audit_tree *alloc_tree(const char *s)
>  {
>  	struct audit_tree *tree;
>  
> -	tree = kmalloc(sizeof(struct audit_tree) + strlen(s) + 1, GFP_KERNEL);
> +	tree = kmalloc(struct_size(tree, pathname, strlen(s) + 1), GFP_KERNEL);
>  	if (tree) {
>  		refcount_set(&tree->count, 1);
>  		tree->goner = 0;
> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index 4173e771650c..19352820b274 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -637,7 +637,7 @@ static struct audit_rule_data *audit_krule_to_data(struct audit_krule *krule)
>  	void *bufp;
>  	int i;
>  
> -	data = kmalloc(sizeof(*data) + krule->buflen, GFP_KERNEL);
> +	data = kmalloc(struct_size(data, buf, krule->buflen), GFP_KERNEL);

Why don't you also transform the zero-length array in struct
audit_rule_data into a flexible-array member:

508 struct audit_rule_data {
509         __u32           flags;  /* AUDIT_PER_{TASK,CALL}, AUDIT_PREPEND */
510         __u32           action; /* AUDIT_NEVER, AUDIT_POSSIBLE, AUDIT_ALWAYS */
511         __u32           field_count;
512         __u32           mask[AUDIT_BITMASK_SIZE]; /* syscall(s) affected */
513         __u32           fields[AUDIT_MAX_FIELDS];
514         __u32           values[AUDIT_MAX_FIELDS];
515         __u32           fieldflags[AUDIT_MAX_FIELDS];
516         __u32           buflen; /* total length of string fields */
517         char            buf[0]; /* string fields buffer */
518 };

Thanks
--
Gustavo

>  	if (unlikely(!data))
>  		return NULL;
>  	memset(data, 0, sizeof(*data));
> -- 
> 2.17.1
> 
> 
> 
> 

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit

Re: [PATCH -next] audit: use struct_size() helper in kmalloc()

[Gustavo A. R. Silva]

On Tue, Dec 14, 2021 at 11:54:48AM -0600, Gustavo A. R. Silva wrote:
> On Tue, Dec 14, 2021 at 07:48:54PM +0800, Xiu Jianfeng wrote:
> > Make use of struct_size() helper instead of an open-coded calucation.
> > 
> > Link: https://github.com/KSPP/linux/issues/160
> > Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>
> > ---
> >  kernel/audit.c       | 2 +-
> >  kernel/audit_tree.c  | 2 +-
> >  kernel/auditfilter.c | 2 +-
> >  3 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index d4084751cfe6..f33028578c60 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c

This could use struct_size(), too:

1461                 audit_send_reply(skb, seq, AUDIT_SIGNAL_INFO, 0, 0,
1462                                  sig_data, sizeof(*sig_data) + len);

> > @@ -1446,7 +1446,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
> >  			if (err)
> >  				return err;
> >  		}
> > -		sig_data = kmalloc(sizeof(*sig_data) + len, GFP_KERNEL);
> > +		sig_data = kmalloc(struct_size(sig_data, ctx, len), GFP_KERNEL);
> >  		if (!sig_data) {
> >  			if (audit_sig_sid)
> >  				security_release_secctx(ctx, len);
> > diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
> > index 72324afcffef..e7315d487163 100644
> > --- a/kernel/audit_tree.c
> > +++ b/kernel/audit_tree.c
> > @@ -94,7 +94,7 @@ static struct audit_tree *alloc_tree(const char *s)
> >  {
> >  	struct audit_tree *tree;
> >  
> > -	tree = kmalloc(sizeof(struct audit_tree) + strlen(s) + 1, GFP_KERNEL);
> > +	tree = kmalloc(struct_size(tree, pathname, strlen(s) + 1), GFP_KERNEL);
> >  	if (tree) {
> >  		refcount_set(&tree->count, 1);
> >  		tree->goner = 0;
> > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> > index 4173e771650c..19352820b274 100644
> > --- a/kernel/auditfilter.c
> > +++ b/kernel/auditfilter.c

Also, in this same file the following piece of code could use
struct_size(), too:

1093                         skb = audit_make_reply(seq, AUDIT_LIST_RULES, 0, 1,
1094                                                data,
1095                                                sizeof(*data) + data->buflen);

Thanks
--
Gustavo

> > @@ -637,7 +637,7 @@ static struct audit_rule_data *audit_krule_to_data(struct audit_krule *krule)
> >  	void *bufp;
> >  	int i;
> >  
> > -	data = kmalloc(sizeof(*data) + krule->buflen, GFP_KERNEL);
> > +	data = kmalloc(struct_size(data, buf, krule->buflen), GFP_KERNEL);
> 
> Why don't you also transform the zero-length array in struct
> audit_rule_data into a flexible-array member:
> 
> 508 struct audit_rule_data {
> 509         __u32           flags;  /* AUDIT_PER_{TASK,CALL}, AUDIT_PREPEND */
> 510         __u32           action; /* AUDIT_NEVER, AUDIT_POSSIBLE, AUDIT_ALWAYS */
> 511         __u32           field_count;
> 512         __u32           mask[AUDIT_BITMASK_SIZE]; /* syscall(s) affected */
> 513         __u32           fields[AUDIT_MAX_FIELDS];
> 514         __u32           values[AUDIT_MAX_FIELDS];
> 515         __u32           fieldflags[AUDIT_MAX_FIELDS];
> 516         __u32           buflen; /* total length of string fields */
> 517         char            buf[0]; /* string fields buffer */
> 518 };
> 
> Thanks
> --
> Gustavo
> 
> >  	if (unlikely(!data))
> >  		return NULL;
> >  	memset(data, 0, sizeof(*data));
> > -- 
> > 2.17.1
> > 
> > 
> > 
> > 

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit

Re: [PATCH -next] audit: use struct_size() helper in kmalloc()

[Paul Moore]

On Tue, Dec 14, 2021 at 6:48 AM Xiu Jianfeng <xiujianfeng@huawei.com> wrote:
>
> Make use of struct_size() helper instead of an open-coded calucation.
>
> Link: https://github.com/KSPP/linux/issues/160
> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>
> ---
>  kernel/audit.c       | 2 +-
>  kernel/audit_tree.c  | 2 +-
>  kernel/auditfilter.c | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)

Merged into audit/next, thanks!

-- 
paul moore
www.paul-moore.com

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit

Re: [PATCH -next] audit: use struct_size() helper in kmalloc()

[xiujianfeng]

在 2021/12/15 1:54, Gustavo A. R. Silva 写道:
> On Tue, Dec 14, 2021 at 07:48:54PM +0800, Xiu Jianfeng wrote:
>> Make use of struct_size() helper instead of an open-coded calucation.
>>
>> Link: https://github.com/KSPP/linux/issues/160
>> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>
>> ---
>>   kernel/audit.c       | 2 +-
>>   kernel/audit_tree.c  | 2 +-
>>   kernel/auditfilter.c | 2 +-
>>   3 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/audit.c b/kernel/audit.c
>> index d4084751cfe6..f33028578c60 100644
>> --- a/kernel/audit.c
>> +++ b/kernel/audit.c
>> @@ -1446,7 +1446,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>>   			if (err)
>>   				return err;
>>   		}
>> -		sig_data = kmalloc(sizeof(*sig_data) + len, GFP_KERNEL);
>> +		sig_data = kmalloc(struct_size(sig_data, ctx, len), GFP_KERNEL);
>>   		if (!sig_data) {
>>   			if (audit_sig_sid)
>>   				security_release_secctx(ctx, len);
>> diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
>> index 72324afcffef..e7315d487163 100644
>> --- a/kernel/audit_tree.c
>> +++ b/kernel/audit_tree.c
>> @@ -94,7 +94,7 @@ static struct audit_tree *alloc_tree(const char *s)
>>   {
>>   	struct audit_tree *tree;
>>   
>> -	tree = kmalloc(sizeof(struct audit_tree) + strlen(s) + 1, GFP_KERNEL);
>> +	tree = kmalloc(struct_size(tree, pathname, strlen(s) + 1), GFP_KERNEL);
>>   	if (tree) {
>>   		refcount_set(&tree->count, 1);
>>   		tree->goner = 0;
>> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
>> index 4173e771650c..19352820b274 100644
>> --- a/kernel/auditfilter.c
>> +++ b/kernel/auditfilter.c
>> @@ -637,7 +637,7 @@ static struct audit_rule_data *audit_krule_to_data(struct audit_krule *krule)
>>   	void *bufp;
>>   	int i;
>>   
>> -	data = kmalloc(sizeof(*data) + krule->buflen, GFP_KERNEL);
>> +	data = kmalloc(struct_size(data, buf, krule->buflen), GFP_KERNEL);
> Why don't you also transform the zero-length array in struct
> audit_rule_data into a flexible-array member:
>
> 508 struct audit_rule_data {
> 509         __u32           flags;  /* AUDIT_PER_{TASK,CALL}, AUDIT_PREPEND */
> 510         __u32           action; /* AUDIT_NEVER, AUDIT_POSSIBLE, AUDIT_ALWAYS */
> 511         __u32           field_count;
> 512         __u32           mask[AUDIT_BITMASK_SIZE]; /* syscall(s) affected */
> 513         __u32           fields[AUDIT_MAX_FIELDS];
> 514         __u32           values[AUDIT_MAX_FIELDS];
> 515         __u32           fieldflags[AUDIT_MAX_FIELDS];
> 516         __u32           buflen; /* total length of string fields */
> 517         char            buf[0]; /* string fields buffer */
> 518 };
>
> Thanks
> --
> Gustavo
thank you, I will add it in the v2 patch.
>>   	if (unlikely(!data))
>>   		return NULL;
>>   	memset(data, 0, sizeof(*data));
>> -- 
>> 2.17.1
>>
>>
>>
>>
> .


--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit

Re: [PATCH -next] audit: use struct_size() helper in kmalloc()

[xiujianfeng]

在 2021/12/15 2:10, Gustavo A. R. Silva 写道:
> On Tue, Dec 14, 2021 at 11:54:48AM -0600, Gustavo A. R. Silva wrote:
>> On Tue, Dec 14, 2021 at 07:48:54PM +0800, Xiu Jianfeng wrote:
>>> Make use of struct_size() helper instead of an open-coded calucation.
>>>
>>> Link: https://github.com/KSPP/linux/issues/160
>>> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>
>>> ---
>>>   kernel/audit.c       | 2 +-
>>>   kernel/audit_tree.c  | 2 +-
>>>   kernel/auditfilter.c | 2 +-
>>>   3 files changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/kernel/audit.c b/kernel/audit.c
>>> index d4084751cfe6..f33028578c60 100644
>>> --- a/kernel/audit.c
>>> +++ b/kernel/audit.c
> This could use struct_size(), too:
>
> 1461                 audit_send_reply(skb, seq, AUDIT_SIGNAL_INFO, 0, 0,
> 1462                                  sig_data, sizeof(*sig_data) + len);
>
>>> @@ -1446,7 +1446,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>>>   			if (err)
>>>   				return err;
>>>   		}
>>> -		sig_data = kmalloc(sizeof(*sig_data) + len, GFP_KERNEL);
>>> +		sig_data = kmalloc(struct_size(sig_data, ctx, len), GFP_KERNEL);
>>>   		if (!sig_data) {
>>>   			if (audit_sig_sid)
>>>   				security_release_secctx(ctx, len);
>>> diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
>>> index 72324afcffef..e7315d487163 100644
>>> --- a/kernel/audit_tree.c
>>> +++ b/kernel/audit_tree.c
>>> @@ -94,7 +94,7 @@ static struct audit_tree *alloc_tree(const char *s)
>>>   {
>>>   	struct audit_tree *tree;
>>>   
>>> -	tree = kmalloc(sizeof(struct audit_tree) + strlen(s) + 1, GFP_KERNEL);
>>> +	tree = kmalloc(struct_size(tree, pathname, strlen(s) + 1), GFP_KERNEL);
>>>   	if (tree) {
>>>   		refcount_set(&tree->count, 1);
>>>   		tree->goner = 0;
>>> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
>>> index 4173e771650c..19352820b274 100644
>>> --- a/kernel/auditfilter.c
>>> +++ b/kernel/auditfilter.c
> Also, in this same file the following piece of code could use
> struct_size(), too:
>
> 1093                         skb = audit_make_reply(seq, AUDIT_LIST_RULES, 0, 1,
> 1094                                                data,
> 1095                                                sizeof(*data) + data->buflen);
>
> Thanks
> --
> Gustavo
thanks, I missed these two places, I will add them in the v2 patch.
>>> @@ -637,7 +637,7 @@ static struct audit_rule_data *audit_krule_to_data(struct audit_krule *krule)
>>>   	void *bufp;
>>>   	int i;
>>>   
>>> -	data = kmalloc(sizeof(*data) + krule->buflen, GFP_KERNEL);
>>> +	data = kmalloc(struct_size(data, buf, krule->buflen), GFP_KERNEL);
>> Why don't you also transform the zero-length array in struct
>> audit_rule_data into a flexible-array member:
>>
>> 508 struct audit_rule_data {
>> 509         __u32           flags;  /* AUDIT_PER_{TASK,CALL}, AUDIT_PREPEND */
>> 510         __u32           action; /* AUDIT_NEVER, AUDIT_POSSIBLE, AUDIT_ALWAYS */
>> 511         __u32           field_count;
>> 512         __u32           mask[AUDIT_BITMASK_SIZE]; /* syscall(s) affected */
>> 513         __u32           fields[AUDIT_MAX_FIELDS];
>> 514         __u32           values[AUDIT_MAX_FIELDS];
>> 515         __u32           fieldflags[AUDIT_MAX_FIELDS];
>> 516         __u32           buflen; /* total length of string fields */
>> 517         char            buf[0]; /* string fields buffer */
>> 518 };
>>
>> Thanks
>> --
>> Gustavo
>>
>>>   	if (unlikely(!data))
>>>   		return NULL;
>>>   	memset(data, 0, sizeof(*data));
>>> -- 
>>> 2.17.1
>>>
>>>
>>>
>>>
> .


--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit

Re: [PATCH -next] audit: use struct_size() helper in kmalloc()

[xiujianfeng]

在 2021/12/15 6:47, Paul Moore 写道:
> On Tue, Dec 14, 2021 at 6:48 AM Xiu Jianfeng <xiujianfeng@huawei.com> wrote:
>> Make use of struct_size() helper instead of an open-coded calucation.
>>
>> Link: https://github.com/KSPP/linux/issues/160
>> Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>
>> ---
>>   kernel/audit.c       | 2 +-
>>   kernel/audit_tree.c  | 2 +-
>>   kernel/auditfilter.c | 2 +-
>>   3 files changed, 3 insertions(+), 3 deletions(-)
> Merged into audit/next, thanks!
thank you, but I missed two places, do you mind if I send a v2 patch?


--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit