linux-kernel - Re: [PATCH] USB:bugfix a controller halt error

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: <2023072629-unblended-perky-4732@gregkh>
Date:   Wed, 26 Jul 2023 09:18:15 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     liulongfang <liulongfang@...wei.com>
Cc:     linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] USB:bugfix a controller halt error

On Wed, Jul 26, 2023 at 02:44:01PM +0800, liulongfang wrote:
> On 2023/7/21 19:08, Greg KH Wrote:
> > On Fri, Jul 21, 2023 at 06:00:15PM +0800, liulongfang wrote:
> >> On systems that use ECC memory. The ECC error of the memory will
> >> cause the USB controller to halt. It causes the usb_control_msg()
> >> operation to fail.
> > 
> > Why does ECC memory matter here?
> >
> 
> This is a test conducted under a special test scenario.
> ECC memory errors are caused by some test tools.

What memory is failing, and why does just this single check matter in
the whole kernel?

If hardware is broken, and failing, it's not the job of the kernel to
protect against that, is it?  Shouldn't the ECC memory controller have
properly notified the kernel of the fault and reset the machine because
it is now in an undetermined state?

> > Are you sure this is correct?  How was this tested?  Seems to me that
> > this will still return "success" if this code path ever happens, what am
> 
> You are right. I made a patch error here. The code modification should be like this:
> if (r < 0) {
> 	retval = r;
> 	kfree(buf);
> 	goto fail;
> }

This means that you didn't test this change at all, so I don't really
think it is needed :(

thanks,

greg k-h