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
