Kees Cook keescook@chromium.org 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