netdev - RE: [PATCH net] 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] [day] [month] [year] [list]

Date:   Mon, 19 Sep 2016 13:30:19 +0000
From:   "Mintz, Yuval" <Yuval.Mintz@...ium.com>
To:     David Laight <David.Laight@...LAB.COM>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH net] qed: Fix stack corruption on probe

> > -	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(p_ver->name[i * sizeof(u32)]);
> >  		*(__be32 *)&p_drv_version->name[i * sizeof(u32)] = val;
> >  	}
> 
> That change fails the 'sanity' test.
> It looks like it is copying a string that might by byteswapped into host order.
> I'm guessing that the target p_drv_version->name[] is char [].
> The cpu_to_be32() call is only correct if p_ver->name[] is 32bit.
> But you are (now) indexing it with the character offset.
> So the data copied cannot be right.

Crap. Thanks for catching this up; I'll send v2 shortly.

> I also suspect it should be be32_to_cpu().
Actually, the conversion is correct as-is - data is provided by driver at
host order and should be sent to management firmware as BE.