linux-kernel - Re: [PATCH 1/1] drivers:net:ethernet:adi:bfin_mac: Use FIELD

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]

Date:   Sun, 23 Apr 2017 22:56:38 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Karim Eshapa <karim.eshapa@...il.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Philippe Reynes <tremyfr@...il.com>,
        Antonio Quartulli <a@...table.cc>,
        Florian Westphal <fw@...len.de>,
        LABBE Corentin <clabbe.montjoie@...il.com>,
        Eric Dumazet <edumazet@...gle.com>,
        Jarod Wilson <jarod@...hat.com>,
        "adi-buildroot-devel@...ts.sourceforge.net" 
        <adi-buildroot-devel@...ts.sourceforge.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1] drivers:net:ethernet:adi:bfin_mac: Use FIELD_SIZEOF
 defined kernel macro

Hi Karim,

On Sun, Apr 23, 2017 at 8:02 PM, Karim Eshapa <karim.eshapa@...il.com> wrote:
> Use FIELD_SIZEOF defined kernel macro kernel.h
>
> Signed-off-by: Karim Eshapa <karim.eshapa@...il.com>
> ---
>  drivers/net/ethernet/adi/bfin_mac.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/adi/bfin_mac.c b/drivers/net/ethernet/adi/bfin_mac.c
> index a9ac58c..60346e0 100644
> --- a/drivers/net/ethernet/adi/bfin_mac.c
> +++ b/drivers/net/ethernet/adi/bfin_mac.c
> @@ -452,10 +452,14 @@ static irqreturn_t bfin_mac_wake_interrupt(int irq, void *dev_id)
>  static void bfin_mac_ethtool_getdrvinfo(struct net_device *dev,
>                                         struct ethtool_drvinfo *info)
>  {
> -       strlcpy(info->driver, KBUILD_MODNAME, sizeof(info->driver));
> -       strlcpy(info->version, DRV_VERSION, sizeof(info->version));
> -       strlcpy(info->fw_version, "N/A", sizeof(info->fw_version));
> -       strlcpy(info->bus_info, dev_name(&dev->dev), sizeof(info->bus_info));
> +       strlcpy(info->driver, KBUILD_MODNAME, FIELD_SIZEOF(
> +               struct ethtool_drvinfo, driver));

IMHO this makes the code less safe and less future-proof.
What if the type of info is ever changed?
There's no safety check to validate that the FIELD_SIZEOF() operates on the
same data as the strlcpy() destination.

> +       strlcpy(info->version, DRV_VERSION, FIELD_SIZEOF(
> +               struct ethtool_drvinfo, version));
> +       strlcpy(info->fw_version, "N/A", FIELD_SIZEOF(
> +               struct ethtool_drvinfo, fw_version));
> +       strlcpy(info->bus_info, dev_name(&dev->dev), FIELD_SIZEOF(
> +               struct ethtool_drvinfo, bus_info));
>  }
>
>  static void bfin_mac_ethtool_getwol(struct net_device *dev,
> @@ -785,7 +789,7 @@ static int bfin_mac_hwtstamp_get(struct net_device *netdev,
>         struct bfin_mac_local *lp = netdev_priv(netdev);
>
>         return copy_to_user(ifr->ifr_data, &lp->stamp_cfg,
> -                           sizeof(lp->stamp_cfg)) ?
> +                           FILD_SIZEOF(struct bfin_mac_local, stamp_cfg)) ?

As the kbuild test robot already told you, this doesn't compile.
Please try to (at least) compile the code before sending patches.
Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds