Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 26 Jul 2023 09:18:15 +0200
From:   Greg KH <gregkh@linuxfoundation.org>
To:     liulongfang <liulongfang@huawei.com>
Cc:     linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] USB:bugfix a controller halt error
Message-ID: <2023072629-unblended-perky-4732@gregkh>
References: <20230721100015.27124-1-liulongfang@huawei.com>
 <2023072153-module-wannabe-5637@gregkh>
 <bc2cd818-c7ad-9061-29ce-f5390d44d8ab@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <bc2cd818-c7ad-9061-29ce-f5390d44d8ab@huawei.com>
Precedence: bulk

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