linux-kernel - RE: [PATCH v3] misc: mei: client.c: fix problem of return '-EOVERFLOW' in mei_cl

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]

Message-ID: <CY5PR11MB6366792D6D72FBC8D4F7DC06ED85A@CY5PR11MB6366.namprd11.prod.outlook.com>
Date:   Tue, 5 Dec 2023 05:30:11 +0000
From:   "Usyskin, Alexander" <alexander.usyskin@...el.com>
To:     Su Hui <suhui@...china.com>, Greg KH <gregkh@...uxfoundation.org>,
        "Dan Carpenter" <dan.carpenter@...aro.org>
CC:     "Winkler, Tomas" <tomas.winkler@...el.com>,
        "arnd@...db.de" <arnd@...db.de>,
        "nathan@...nel.org" <nathan@...nel.org>,
        "ndesaulniers@...gle.com" <ndesaulniers@...gle.com>,
        "Rix, Tom" <trix@...hat.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "llvm@...ts.linux.dev" <llvm@...ts.linux.dev>,
        "kernel-janitors@...r.kernel.org" <kernel-janitors@...r.kernel.org>
Subject: RE: [PATCH v3] misc: mei: client.c: fix problem of return
 '-EOVERFLOW' in mei_cl_write

> >>>> diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
> >>>> index 7ea80779a0e2..0489bec4fded 100644
> >>>> --- a/drivers/misc/mei/client.c
> >>>> +++ b/drivers/misc/mei/client.c
> >>>> @@ -2033,7 +2033,7 @@ ssize_t mei_cl_write(struct mei_cl *cl, struct
> mei_cl_cb *cb, unsigned long time
> >>>>   	hbuf_slots = mei_hbuf_empty_slots(dev);
> >>>>   	if (hbuf_slots < 0) {
> >>>>   		rets = -EOVERFLOW;
> >>>> -		goto out;
> >>>> +		goto err;
> >>> Please prove that this is correct, as based on the code logic, it seems
> >>> very wrong.  I can't take this unless the code is tested properly.
> >> Hi Greg,
> >>
> >> When Su Hui sent the v2 patch you sent an auto response about adding
> >> stable to the CC list.
> >> https://lore.kernel.org/all/2023112042-napped-snoring-b766@gregkh/
> >>
> >> However, it appears that you still applied the v2 patch.  It's in
> >> linux-next as commit ee6236027218 ("misc: mei: client.c: fix problem of
> >> return '-EOVERFLOW' in mei_cl_write").
> >>
> >> When I use `git am` to apply this patch, then it doesn't apply.  However,
> >> when I use cat email.txt | patch -p1 then it tries to reverse the patch
> >> and apply it to a different function.
> > Odd, I missed that I had already applied the first one, nevermind, that
> > one is correct, this one was wrong :)
> 
> Hi,
> 
> Oh, sorry...
> I'm not familiar with mei device, I send this v3 patch because of Sasha'
> s suggestion.[1]
> Could Sasha give some advice about this ?
> Thanks a lot :)
> 
> https://lore.kernel.org/all/CY5PR11MB63668F464A281A239FA12B6AEDBDA@C
> Y5PR11MB6366.namprd11.prod.outlook.com/
> 

When we have overflow from the hbuf - that means the HW bookkeeping
is in mess (write pointer is before read pointer).
There is no point in continuing in the write flow, we should exit with error and
let driver to re-initialize HW.
I've never seen this error in the wild, so it is very hard to test it.

-- 
Thanks,
Sasha