netdev - Re: [PATCH net v2] qed: Fix stack corruption on probe

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]

Message-ID: <BL2PR07MB23066CD6432BBDE74937E6E08DF40@BL2PR07MB2306.namprd07.prod.outlook.com>
Date:   Mon, 19 Sep 2016 17:24:46 +0000
From:   "Mintz, Yuval" <Yuval.Mintz@...ium.com>
To:     David Laight <David.Laight@...LAB.COM>,
        'Yuval Mintz' <Yuval.Mintz@...gic.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH net v2] qed: Fix stack corruption on probe

> > Commit fe56b9e6a8d95 ("qed: Add module with basic common support")
> > has introduced a stack corruption during probe, where filling a
> > local struct with data to be sent to management firmware is incorrectly
> > filled; The data is written outside of the struct and corrupts
> > the stack.
> > 
> > @@ -1153,8 +1153,8 @@ qed_mcp_send_drv_version(struct qed_hwfn *p_hwfn,
> >        p_drv_version = &union_data.drv_version;
> >        p_drv_version->version = p_ver->version;
> > 
> > -     for (i = 0; i < MCP_DRV_VER_STR_SIZE - 1; i += 4) {
> > -             val = cpu_to_be32(p_ver->name[i]);
> > +     for (i = 0; i < (MCP_DRV_VER_STR_SIZE - 4) / sizeof(u32); i++) {
> > +             val = cpu_to_be32(*((u32 *)&p_ver->name[i * sizeof(u32)]));
> >                *(__be32 *)&p_drv_version->name[i * sizeof(u32)] = val;
> >        }
> 
> Or the much simpler:
>      /* Byteswap driver name */
>         for (i = 0; i < MCP_DRV_VER_STR_SIZE; i++)
>                 p_drv_version->name[i ^ 3] = p_ver->name[i];

You're suggesting an unconditional swap; This will break
the message on a big endian machine [we'll swap needlessly].