2021-12-15 03:03:28

by Xiu Jianfeng

[permalink] [raw]
Subject: [PATCH -next, v2] audit: use struct_size() helper in kmalloc()

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

Link: https://github.com/KSPP/linux/issues/160
Signed-off-by: Xiu Jianfeng <[email protected]>
---
include/uapi/linux/audit.h | 2 +-
kernel/audit.c | 4 ++--
kernel/audit_tree.c | 2 +-
kernel/auditfilter.c | 4 ++--
4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 9176a095fefc..8eda133ca4c1 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -514,7 +514,7 @@ struct audit_rule_data {
__u32 values[AUDIT_MAX_FIELDS];
__u32 fieldflags[AUDIT_MAX_FIELDS];
__u32 buflen; /* total length of string fields */
- char buf[0]; /* string fields buffer */
+ char buf[]; /* string fields buffer */
};

#endif /* _UAPI_LINUX_AUDIT_H_ */
diff --git a/kernel/audit.c b/kernel/audit.c
index d4084751cfe6..7778eca34837 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);
@@ -1459,7 +1459,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
security_release_secctx(ctx, len);
}
audit_send_reply(skb, seq, AUDIT_SIGNAL_INFO, 0, 0,
- sig_data, sizeof(*sig_data) + len);
+ sig_data, struct_size(sig_data, ctx, len));
kfree(sig_data);
break;
case AUDIT_TTY_GET: {
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..42d99896e7a6 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));
@@ -1092,7 +1092,7 @@ static void audit_list_rules(int seq, struct sk_buff_head *q)
break;
skb = audit_make_reply(seq, AUDIT_LIST_RULES, 0, 1,
data,
- sizeof(*data) + data->buflen);
+ struct_size(data, buf, data->buflen));
if (skb)
skb_queue_tail(q, skb);
kfree(data);
--
2.17.1



2021-12-15 21:54:27

by Gustavo A. R. Silva

[permalink] [raw]
Subject: Re: [PATCH -next, v2] audit: use struct_size() helper in kmalloc()

On Wed, Dec 15, 2021 at 11:04:20AM +0800, Xiu Jianfeng wrote:
> Make use of struct_size() helper instead of an open-coded calculation.

I think you should also mention the flexible array transformation in
struct audit_rule_data.

Thanks
--
Gustavo

>
> Link: https://github.com/KSPP/linux/issues/160
> Signed-off-by: Xiu Jianfeng <[email protected]>
> ---
> include/uapi/linux/audit.h | 2 +-
> kernel/audit.c | 4 ++--
> kernel/audit_tree.c | 2 +-
> kernel/auditfilter.c | 4 ++--
> 4 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 9176a095fefc..8eda133ca4c1 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -514,7 +514,7 @@ struct audit_rule_data {
> __u32 values[AUDIT_MAX_FIELDS];
> __u32 fieldflags[AUDIT_MAX_FIELDS];
> __u32 buflen; /* total length of string fields */
> - char buf[0]; /* string fields buffer */
> + char buf[]; /* string fields buffer */
> };
>
> #endif /* _UAPI_LINUX_AUDIT_H_ */
> diff --git a/kernel/audit.c b/kernel/audit.c
> index d4084751cfe6..7778eca34837 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);
> @@ -1459,7 +1459,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
> security_release_secctx(ctx, len);
> }
> audit_send_reply(skb, seq, AUDIT_SIGNAL_INFO, 0, 0,
> - sig_data, sizeof(*sig_data) + len);
> + sig_data, struct_size(sig_data, ctx, len));
> kfree(sig_data);
> break;
> case AUDIT_TTY_GET: {
> 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..42d99896e7a6 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));
> @@ -1092,7 +1092,7 @@ static void audit_list_rules(int seq, struct sk_buff_head *q)
> break;
> skb = audit_make_reply(seq, AUDIT_LIST_RULES, 0, 1,
> data,
> - sizeof(*data) + data->buflen);
> + struct_size(data, buf, data->buflen));
> if (skb)
> skb_queue_tail(q, skb);
> kfree(data);
> --
> 2.17.1
>

2021-12-16 02:05:27

by Xiu Jianfeng

[permalink] [raw]
Subject: Re: [PATCH -next, v2] audit: use struct_size() helper in kmalloc()


在 2021/12/16 6:00, Gustavo A. R. Silva 写道:
> On Wed, Dec 15, 2021 at 11:04:20AM +0800, Xiu Jianfeng wrote:
>> Make use of struct_size() helper instead of an open-coded calculation.
> I think you should also mention the flexible array transformation in
> struct audit_rule_data.

thanks,  and due to the previous patch has been merged into linux-next,
a diff patch about struct_size() and

a seperate patch about flexible array will be send.

>
> Thanks
> --
> Gustavo
>
>> Link: https://github.com/KSPP/linux/issues/160
>> Signed-off-by: Xiu Jianfeng <[email protected]>
>> ---
>> include/uapi/linux/audit.h | 2 +-
>> kernel/audit.c | 4 ++--
>> kernel/audit_tree.c | 2 +-
>> kernel/auditfilter.c | 4 ++--
>> 4 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
>> index 9176a095fefc..8eda133ca4c1 100644
>> --- a/include/uapi/linux/audit.h
>> +++ b/include/uapi/linux/audit.h
>> @@ -514,7 +514,7 @@ struct audit_rule_data {
>> __u32 values[AUDIT_MAX_FIELDS];
>> __u32 fieldflags[AUDIT_MAX_FIELDS];
>> __u32 buflen; /* total length of string fields */
>> - char buf[0]; /* string fields buffer */
>> + char buf[]; /* string fields buffer */
>> };
>>
>> #endif /* _UAPI_LINUX_AUDIT_H_ */
>> diff --git a/kernel/audit.c b/kernel/audit.c
>> index d4084751cfe6..7778eca34837 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);
>> @@ -1459,7 +1459,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>> security_release_secctx(ctx, len);
>> }
>> audit_send_reply(skb, seq, AUDIT_SIGNAL_INFO, 0, 0,
>> - sig_data, sizeof(*sig_data) + len);
>> + sig_data, struct_size(sig_data, ctx, len));
>> kfree(sig_data);
>> break;
>> case AUDIT_TTY_GET: {
>> 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..42d99896e7a6 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));
>> @@ -1092,7 +1092,7 @@ static void audit_list_rules(int seq, struct sk_buff_head *q)
>> break;
>> skb = audit_make_reply(seq, AUDIT_LIST_RULES, 0, 1,
>> data,
>> - sizeof(*data) + data->buflen);
>> + struct_size(data, buf, data->buflen));
>> if (skb)
>> skb_queue_tail(q, skb);
>> kfree(data);
>> --
>> 2.17.1
>>
> .

2021-12-16 22:43:02

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH -next, v2] audit: use struct_size() helper in kmalloc()

On Wed, Dec 15, 2021 at 9:05 PM xiujianfeng <[email protected]> wrote:
> 在 2021/12/16 6:00, Gustavo A. R. Silva 写道:
> > On Wed, Dec 15, 2021 at 11:04:20AM +0800, Xiu Jianfeng wrote:
> >> Make use of struct_size() helper instead of an open-coded calculation.
> > I think you should also mention the flexible array transformation in
> > struct audit_rule_data.
>
> thanks, and due to the previous patch has been merged into linux-next,
> a diff patch about struct_size() and
>
> a seperate patch about flexible array will be send.

Please make sure to send it to the audit mailing list as well. I
don't like merging patches that haven't been sent to the mailing list,
it's a bad practice IMO.

--
paul moore
http://www.paul-moore.com