linux-kernel - Re: [PATCH -next] audit: use struct

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8304d0db-c1b1-ceca-4f6f-d1fe5d486a1e@huawei.com>
Date:   Wed, 15 Dec 2021 10:54:44 +0800
From:   xiujianfeng <xiujianfeng@...wei.com>
To:     "Gustavo A. R. Silva" <gustavoars@...nel.org>
CC:     <paul@...l-moore.com>, <eparis@...hat.com>,
        <keescook@...omium.org>, <linux-audit@...hat.com>,
        <linux-kernel@...r.kernel.org>, <wangweiyang2@...wei.com>,
        <linux-hardening@...r.kernel.org>
Subject: Re: [PATCH -next] audit: use struct_size() helper in kmalloc()


在 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@...wei.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
>>
>>
>>
>>
> .