* [PATCH v3] misc: mei: client.c: fix problem of return '-EOVERFLOW' in mei_cl_write
@ 2023-11-28 1:45 Su Hui
2023-12-04 8:00 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Su Hui @ 2023-11-28 1:45 UTC (permalink / raw)
To: alexander.usyskin, tomas.winkler, arnd, gregkh, nathan,
ndesaulniers, trix
Cc: Su Hui, linux-kernel, llvm, kernel-janitors
Clang static analyzer complains that value stored to 'rets' is never
read.Using 'goto err' to go to the error path and fix this problem.
Fixes: 8c8d964ce90f ("mei: move hbuf_depth from the mei device to the hw modules")
Signed-off-by: Su Hui <suhui@nfschina.com>
---
v3:
- using 'goto err' rather than 'buf_len=-EOVERFLOW'.(Thanks to Sasha)
v2:
- split v1 patch to different patches
https://lore.kernel.org/all/20231120095523.178385-2-suhui@nfschina.com/
v1:
https://lore.kernel.org/all/5c98fc07-36a9-92cc-f8d6-c4efdc0c34aa@nfschina.com/
drivers/misc/mei/client.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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;
}
hbuf_len = mei_slots2data(hbuf_slots) & MEI_MSG_MAX_LEN_MASK;
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3] misc: mei: client.c: fix problem of return '-EOVERFLOW' in mei_cl_write
2023-11-28 1:45 [PATCH v3] misc: mei: client.c: fix problem of return '-EOVERFLOW' in mei_cl_write Su Hui
@ 2023-12-04 8:00 ` Greg KH
2023-12-04 13:11 ` Dan Carpenter
0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2023-12-04 8:00 UTC (permalink / raw)
To: Su Hui
Cc: alexander.usyskin, tomas.winkler, arnd, nathan, ndesaulniers,
trix, linux-kernel, llvm, kernel-janitors
On Tue, Nov 28, 2023 at 09:45:08AM +0800, Su Hui wrote:
> Clang static analyzer complains that value stored to 'rets' is never
> read.Using 'goto err' to go to the error path and fix this problem.
>
> Fixes: 8c8d964ce90f ("mei: move hbuf_depth from the mei device to the hw modules")
> Signed-off-by: Su Hui <suhui@nfschina.com>
How was this tested?
> ---
> v3:
> - using 'goto err' rather than 'buf_len=-EOVERFLOW'.(Thanks to Sasha)
>
> v2:
> - split v1 patch to different patches
> https://lore.kernel.org/all/20231120095523.178385-2-suhui@nfschina.com/
>
> v1:
> https://lore.kernel.org/all/5c98fc07-36a9-92cc-f8d6-c4efdc0c34aa@nfschina.com/
>
> drivers/misc/mei/client.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> 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.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] misc: mei: client.c: fix problem of return '-EOVERFLOW' in mei_cl_write
2023-12-04 8:00 ` Greg KH
@ 2023-12-04 13:11 ` Dan Carpenter
2023-12-04 13:17 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2023-12-04 13:11 UTC (permalink / raw)
To: Greg KH
Cc: Su Hui, alexander.usyskin, tomas.winkler, arnd, nathan,
ndesaulniers, trix, linux-kernel, llvm, kernel-janitors
On Mon, Dec 04, 2023 at 09:00:42AM +0100, Greg KH wrote:
> > 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.
So this a kind of confusing thing.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] misc: mei: client.c: fix problem of return '-EOVERFLOW' in mei_cl_write
2023-12-04 13:11 ` Dan Carpenter
@ 2023-12-04 13:17 ` Greg KH
2023-12-05 2:30 ` Su Hui
0 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2023-12-04 13:17 UTC (permalink / raw)
To: Dan Carpenter
Cc: Su Hui, alexander.usyskin, tomas.winkler, arnd, nathan,
ndesaulniers, trix, linux-kernel, llvm, kernel-janitors
On Mon, Dec 04, 2023 at 04:11:31PM +0300, Dan Carpenter wrote:
> On Mon, Dec 04, 2023 at 09:00:42AM +0100, Greg KH wrote:
> > > 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 :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] misc: mei: client.c: fix problem of return '-EOVERFLOW' in mei_cl_write
2023-12-04 13:17 ` Greg KH
@ 2023-12-05 2:30 ` Su Hui
2023-12-05 5:30 ` Usyskin, Alexander
0 siblings, 1 reply; 6+ messages in thread
From: Su Hui @ 2023-12-05 2:30 UTC (permalink / raw)
To: alexander.usyskin, Greg KH, Dan Carpenter
Cc: tomas.winkler, arnd, nathan, ndesaulniers, trix, linux-kernel,
llvm, kernel-janitors
On 2023/12/4 21:17, Greg KH wrote:
> On Mon, Dec 04, 2023 at 04:11:31PM +0300, Dan Carpenter wrote:
>> On Mon, Dec 04, 2023 at 09:00:42AM +0100, Greg KH wrote:
>>>> 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@CY5PR11MB6366.namprd11.prod.outlook.com/
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v3] misc: mei: client.c: fix problem of return '-EOVERFLOW' in mei_cl_write
2023-12-05 2:30 ` Su Hui
@ 2023-12-05 5:30 ` Usyskin, Alexander
0 siblings, 0 replies; 6+ messages in thread
From: Usyskin, Alexander @ 2023-12-05 5:30 UTC (permalink / raw)
To: Su Hui, Greg KH, Dan Carpenter
Cc: Winkler, Tomas, arnd@arndb.de, nathan@kernel.org,
ndesaulniers@google.com, Rix, Tom, linux-kernel@vger.kernel.org,
llvm@lists.linux.dev, kernel-janitors@vger.kernel.org
> >>>> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-12-05 5:30 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-28 1:45 [PATCH v3] misc: mei: client.c: fix problem of return '-EOVERFLOW' in mei_cl_write Su Hui
2023-12-04 8:00 ` Greg KH
2023-12-04 13:11 ` Dan Carpenter
2023-12-04 13:17 ` Greg KH
2023-12-05 2:30 ` Su Hui
2023-12-05 5:30 ` Usyskin, Alexander
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox