usb_control_msg_send() and usb_control_msg_recv() are highly problematic

usb_control_msg_send() and usb_control_msg_recv() are highly problematic

[Oliver Neukum]

Hi,

you probably do not want to hear this. I was out of comission
for the last week weeks and I should have looked at this more closely.

You may notice that usb_control_msg() for times immemorial has been
using GFP_NOIO internally. This is because control messages are needed
in a lot of contexts such as SCSI error handling and runtime PM
that require GFP_NOIO. IIRC at that time we found ourselves unable to
go through all those call chains, so we pulled the nuclear option
and slapped a blanket GFP_NOIO on the function.

Today I got a patch that outright deleted a memory allocation with
GFP_NOIO, so I looked into this. We are making the same mistake
we couldn't fix in the past.
I am afraid the API has to be changed to include memory flags.
And we should do this now while the damage is still within limits.
I am preparing patches.

	Regards
		Oliver

Re: usb_control_msg_send() and usb_control_msg_recv() are highly problematic

[Greg KH]

On Wed, Sep 23, 2020 at 12:59:00PM +0200, Oliver Neukum wrote:
> Hi,
> 
> you probably do not want to hear this. I was out of comission
> for the last week weeks and I should have looked at this more closely.
> 
> You may notice that usb_control_msg() for times immemorial has been
> using GFP_NOIO internally. This is because control messages are needed
> in a lot of contexts such as SCSI error handling and runtime PM
> that require GFP_NOIO. IIRC at that time we found ourselves unable to
> go through all those call chains, so we pulled the nuclear option
> and slapped a blanket GFP_NOIO on the function.
> 
> Today I got a patch that outright deleted a memory allocation with
> GFP_NOIO, so I looked into this. We are making the same mistake
> we couldn't fix in the past.
> I am afraid the API has to be changed to include memory flags.
> And we should do this now while the damage is still within limits.
> I am preparing patches.

How about we always use GPF_NOIO for the calls?  That should be fine and
make it easier, right?

thanks,

greg k-h

Re: usb_control_msg_send() and usb_control_msg_recv() are highly problematic

[Oliver Neukum]

Am Mittwoch, den 23.09.2020, 13:07 +0200 schrieb Greg KH:
> On Wed, Sep 23, 2020 at 12:59:00PM +0200, Oliver Neukum wrote:
> > Hi,
> > 
> > you probably do not want to hear this. I was out of comission
> > for the last week weeks and I should have looked at this more closely.
> > 
> > You may notice that usb_control_msg() for times immemorial has been
> > using GFP_NOIO internally. This is because control messages are needed
> > in a lot of contexts such as SCSI error handling and runtime PM
> > that require GFP_NOIO. IIRC at that time we found ourselves unable to
> > go through all those call chains, so we pulled the nuclear option
> > and slapped a blanket GFP_NOIO on the function.
> > 
> > Today I got a patch that outright deleted a memory allocation with
> > GFP_NOIO, so I looked into this. We are making the same mistake
> > we couldn't fix in the past.
> > I am afraid the API has to be changed to include memory flags.
> > And we should do this now while the damage is still within limits.
> > I am preparing patches.
> 
> How about we always use GPF_NOIO for the calls?  That should be fine and
> make it easier, right?

We are supposed to use GFP_NOIO thousands of times without any reason?
Usually we try to be better citizens in terms of VM allocations.

Of course this works, but do you want an API that is flawed  by design?

	Regards
		Oliver