linux-kernel - Re: [PATCH 2/5] gtp: Improve another size determination in ipv4_pdp

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] [day] [month] [year] [list]

Date:   Thu, 26 Jan 2017 16:11:08 +0300
From:   Alexey Dobriyan <adobriyan@...il.com>
To:     SF Markus Elfring <elfring@...rs.sourceforge.net>
Cc:     netdev <netdev@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Harald Welte <laforge@...monks.org>,
        Johannes Berg <johannes.berg@...el.com>,
        Pablo Neira <pablo@...filter.org>,
        LKML <linux-kernel@...r.kernel.org>,
        kernel-janitors@...r.kernel.org
Subject: Re: [PATCH 2/5] gtp: Improve another size determination in ipv4_pdp_add()

On Thu, Jan 26, 2017 at 1:18 PM, SF Markus Elfring
<elfring@...rs.sourceforge.net> wrote:

> -       pctx = kmalloc(sizeof(struct pdp_ctx), GFP_KERNEL);
> +       pctx = kmalloc(sizeof(*pctx), GFP_KERNEL);

The rule about "sizeof(*p)" style of allocation is bogus and should be
abolished.

Rationale says that

    > The alternative form where struct name is spelled out hurts readability

In terms of line length, yes, "sizeof(*p)" wins most of the time.
However, the former variant clearly show the type of data, so you don't need
to look it up and makes it readily available to follow with tags or equivalent.

    > and introduces an opportunity for a bug when the pointer variable type
    > is changed but the corresponding sizeof that is passed to a memory
    > allocator is not.

The correct way to prevent this kind of mistake is to not return "void
*" pointer
(at least most of the uses use typed allocation)

    #define lmalloc(T, gfp) (T*)_kmalloc(sizeof(T), (gfp))

Bacause of a cast changing type of pointer will be noticed,