linux-kernel - Re: [PATCH] PCI: dwc: Fix a 64bit bug in dw_pcie_ep_raise_msix

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: Fri, 19 Jan 2024 13:06:25 +0100
From: Niklas Cassel <cassel@...nel.org>
To: Dan Carpenter <dan.carpenter@...aro.org>
Cc: Niklas Cassel <Niklas.Cassel@....com>,
	Jingoo Han <jingoohan1@...il.com>,
	Gustavo Pimentel <gustavo.pimentel@...opsys.com>,
	Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
	Lorenzo Pieralisi <lpieralisi@...nel.org>,
	Krzysztof Wilczyński <kw@...ux.com>,
	Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
	"linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"kernel-janitors@...r.kernel.org" <kernel-janitors@...r.kernel.org>
Subject: Re: [PATCH] PCI: dwc: Fix a 64bit bug in dw_pcie_ep_raise_msix_irq()

On Fri, Jan 19, 2024 at 11:25:51AM +0300, Dan Carpenter wrote:
> On Wed, Jan 17, 2024 at 09:21:41PM +0000, Niklas Cassel wrote:
> > Hello Dan,
> > 
> > On Wed, Jan 17, 2024 at 09:32:08PM +0300, Dan Carpenter wrote:
> > > The "msg_addr" variable is u64.  However, the "tbl_offset" is an unsigned
> > 
> > Here you write tbl_offset.
> > 
> > > int.  This means that when the code does
> > > 
> > > 	msg_addr &= ~aligned_offset;
> > > 
> > > it will unintentionally zero out the high 32 bits.  Declare "tbl_offset"
> > 
> > Here you also write tbl_offset.
> > 
> 
> That's so weird...  I can't imagine how that happened.  Do you think it
> could be a Welsh mice situation where forest creatures are changing my
> work when I'm away from my desk?  https://www.youtube.com/shorts/h8gkIbtaaek

Yes, that it the most likely scenario :)

In fact, I think that is what happened with my original patch too...

Because while the C-standards says:

"""
6.5.3.3 Unary arithmetic operators
The result of the unary - operator is the negative of its (promoted) operand. The integer
promotions are performed on the operand, and the result has the promoted type.
"""


Of course, I also remember that implicit integer promotions are only up to
"int" or "unsigned int".

Because of course it is fine to convert types smaller than int implicitly...
but bigger than int? No way! :)


#include <stdio.h>
#include <stdint.h>

void main()
{
        uint16_t val_16 = 0xffff;
        uint8_t mask_8 = 0xf0;

        uint32_t val_32 = 0xffffffff;
        uint16_t mask_16 = 0x00f0;

        uint64_t val_64 = 0xffffffffffffffff;
        uint32_t mask_32 = 0x000000f0;

        uint16_t res_16 = val_16 & ~mask_8;
        uint32_t res_32 = val_32 & ~mask_16;
        uint64_t res_64 = val_64 & ~mask_32;

        printf("16: res: %#llx val: %#llx mask: %#llx\n", res_16, val_16, mask_8);
        printf("32: res: %#llx val: %#llx mask: %#llx\n", res_32, val_32, mask_16);
        printf("64: res: %#llx val: %#llx mask: %#llx\n", res_64, val_64, mask_32);
}

output:
16: res: 0xff0f val: 0xffff mask: 0xf0
32: res: 0xffffff0f val: 0xffffffff mask: 0xf0
64: res: 0xffffff0f val: 0xffffffffffffffff mask: 0xf0

(Silly me for not also reading 6.3.1.1...)


Kind regards,
Niklas