Re: [PATCH 19/32] afs: Use mem_to_flex_dup() with struct afs_acl

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Kees Cook <keescook@xxxxxxxxxxxx> wrote:

>  struct afs_acl {
> -	u32	size;
> -	u8	data[];
> +	DECLARE_FLEX_ARRAY_ELEMENTS_COUNT(u32, size);
> +	DECLARE_FLEX_ARRAY_ELEMENTS(u8, data);
>  };

Oof...  That's really quite unpleasant syntax.  Is it not possible to have
mem_to_flex_dup() and friends work without that?  You are telling them the
fields they have to fill in.

> +	struct afs_acl *acl = NULL;
>  
> -	acl = kmalloc(sizeof(*acl) + size, GFP_KERNEL);
> -	if (!acl) {
> +	if (mem_to_flex_dup(&acl, buffer, size, GFP_KERNEL)) {

Please don't do that.  Either do:

	acl = mem_to_flex_dup(buffer, size, GFP_KERNEL);
	if (!acl)

or:

	acl = mem_to_flex_dup(buffer, size, GFP_KERNEL);
	if (IS_ERR(acl))

Please especially don't make it that an apparent 'true' return indicates an
error.  If you absolutely must return the acl pointer through the argument
list (presumably because it's actually a macro), make it return false on
failure:

	if (!mem_to_flex_dup(&acl, buffer, size, GFP_KERNEL))

or return and explicitly check for an error code:

	if (mem_to_flex_dup(&acl, buffer, size, GFP_KERNEL) < 0)

or:

	ret = mem_to_flex_dup(&acl, buffer, size, GFP_KERNEL);
	if (ret < 0)

(or use != 0 rather than < 0)

David




[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux