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 <[email protected]>
---
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
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 <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
--
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 <[email protected]>
> ---
> 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
>
>
>
>
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 <[email protected]>
> > ---
> > 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
> >
> >
> >
> >
On Tue, Dec 14, 2021 at 6:48 AM Xiu Jianfeng <[email protected]> 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 <[email protected]>
> ---
> 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
http://www.paul-moore.com
?? 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 <[email protected]>
>> ---
>> 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
>>
>>
>>
>>
> .
?? 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 <[email protected]>
>>> ---
>>> 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
>>>
>>>
>>>
>>>
> .
在 2021/12/15 6:47, Paul Moore 写道:
> On Tue, Dec 14, 2021 at 6:48 AM Xiu Jianfeng <[email protected]> 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 <[email protected]>
>> ---
>> 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?