All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] USB: usblp: fix DMA to stack
@ 2021-01-04 14:53 Johan Hovold
  2021-01-04 14:59 ` Greg Kroah-Hartman
  2021-01-04 17:37 ` Pete Zaitcev
  0 siblings, 2 replies; 7+ messages in thread
From: Johan Hovold @ 2021-01-04 14:53 UTC (permalink / raw)
  To: Pete Zaitcev
  Cc: Greg Kroah-Hartman, linux-usb, Dan Carpenter, Johan Hovold, stable

Stack-allocated buffers cannot be used for DMA (on all architectures).

Replace the HP-channel macro with a helper function that allocates a
dedicated transfer buffer so that it can continue to be used with
arguments from the stack.

Note that the buffer is cleared on allocation as usblp_ctrl_msg()
returns success also on short transfers (the buffer is only used for
debugging).

Cc: stable@vger.kernel.org
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/class/usblp.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/class/usblp.c b/drivers/usb/class/usblp.c
index 67cbd42421be..134dc2005ce9 100644
--- a/drivers/usb/class/usblp.c
+++ b/drivers/usb/class/usblp.c
@@ -274,8 +274,25 @@ static int usblp_ctrl_msg(struct usblp *usblp, int request, int type, int dir, i
 #define usblp_reset(usblp)\
 	usblp_ctrl_msg(usblp, USBLP_REQ_RESET, USB_TYPE_CLASS, USB_DIR_OUT, USB_RECIP_OTHER, 0, NULL, 0)
 
-#define usblp_hp_channel_change_request(usblp, channel, buffer) \
-	usblp_ctrl_msg(usblp, USBLP_REQ_HP_CHANNEL_CHANGE_REQUEST, USB_TYPE_VENDOR, USB_DIR_IN, USB_RECIP_INTERFACE, channel, buffer, 1)
+static int usblp_hp_channel_change_request(struct usblp *usblp, int channel, u8 *new_channel)
+{
+	u8 *buf;
+	int ret;
+
+	buf = kzalloc(1, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = usblp_ctrl_msg(usblp, USBLP_REQ_HP_CHANNEL_CHANGE_REQUEST,
+			USB_TYPE_VENDOR, USB_DIR_IN, USB_RECIP_INTERFACE,
+			channel, buf, 1);
+	if (ret == 0)
+		*new_channel = buf[0];
+
+	kfree(buf);
+
+	return ret;
+}
 
 /*
  * See the description for usblp_select_alts() below for the usage
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] USB: usblp: fix DMA to stack
  2021-01-04 14:53 [PATCH] USB: usblp: fix DMA to stack Johan Hovold
@ 2021-01-04 14:59 ` Greg Kroah-Hartman
  2021-01-04 15:11   ` Johan Hovold
  2021-01-04 17:37 ` Pete Zaitcev
  1 sibling, 1 reply; 7+ messages in thread
From: Greg Kroah-Hartman @ 2021-01-04 14:59 UTC (permalink / raw)
  To: Johan Hovold; +Cc: Pete Zaitcev, linux-usb, Dan Carpenter, stable

On Mon, Jan 04, 2021 at 03:53:02PM +0100, Johan Hovold wrote:
> Stack-allocated buffers cannot be used for DMA (on all architectures).
> 
> Replace the HP-channel macro with a helper function that allocates a
> dedicated transfer buffer so that it can continue to be used with
> arguments from the stack.
> 
> Note that the buffer is cleared on allocation as usblp_ctrl_msg()
> returns success also on short transfers (the buffer is only used for
> debugging).
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/usb/class/usblp.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/class/usblp.c b/drivers/usb/class/usblp.c
> index 67cbd42421be..134dc2005ce9 100644
> --- a/drivers/usb/class/usblp.c
> +++ b/drivers/usb/class/usblp.c
> @@ -274,8 +274,25 @@ static int usblp_ctrl_msg(struct usblp *usblp, int request, int type, int dir, i
>  #define usblp_reset(usblp)\
>  	usblp_ctrl_msg(usblp, USBLP_REQ_RESET, USB_TYPE_CLASS, USB_DIR_OUT, USB_RECIP_OTHER, 0, NULL, 0)
>  
> -#define usblp_hp_channel_change_request(usblp, channel, buffer) \
> -	usblp_ctrl_msg(usblp, USBLP_REQ_HP_CHANNEL_CHANGE_REQUEST, USB_TYPE_VENDOR, USB_DIR_IN, USB_RECIP_INTERFACE, channel, buffer, 1)
> +static int usblp_hp_channel_change_request(struct usblp *usblp, int channel, u8 *new_channel)
> +{
> +	u8 *buf;
> +	int ret;
> +
> +	buf = kzalloc(1, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	ret = usblp_ctrl_msg(usblp, USBLP_REQ_HP_CHANNEL_CHANGE_REQUEST,
> +			USB_TYPE_VENDOR, USB_DIR_IN, USB_RECIP_INTERFACE,
> +			channel, buf, 1);
> +	if (ret == 0)
> +		*new_channel = buf[0];
> +
> +	kfree(buf);
> +
> +	return ret;
> +}

Wow, no one uses this driver anymore it seems, this should have
triggered a runtime warning on newer kernels :(

Thanks for this, will queue it up soon.

greg k-h

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] USB: usblp: fix DMA to stack
  2021-01-04 14:59 ` Greg Kroah-Hartman
@ 2021-01-04 15:11   ` Johan Hovold
  2021-01-04 15:33     ` Michael Sweet
  0 siblings, 1 reply; 7+ messages in thread
From: Johan Hovold @ 2021-01-04 15:11 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Johan Hovold, Pete Zaitcev, linux-usb, Dan Carpenter, stable

On Mon, Jan 04, 2021 at 03:59:02PM +0100, Greg Kroah-Hartman wrote:
> On Mon, Jan 04, 2021 at 03:53:02PM +0100, Johan Hovold wrote:
> > Stack-allocated buffers cannot be used for DMA (on all architectures).
> > 
> > Replace the HP-channel macro with a helper function that allocates a
> > dedicated transfer buffer so that it can continue to be used with
> > arguments from the stack.

> Wow, no one uses this driver anymore it seems, this should have
> triggered a runtime warning on newer kernels :(

This helper is only used by the IOCNR_HP_SET_CHANNEL ioctl so perhaps
it's just that one which isn't used much.

Johan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] USB: usblp: fix DMA to stack
  2021-01-04 15:11   ` Johan Hovold
@ 2021-01-04 15:33     ` Michael Sweet
  2021-01-04 15:48       ` Johan Hovold
  0 siblings, 1 reply; 7+ messages in thread
From: Michael Sweet @ 2021-01-04 15:33 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Greg Kroah-Hartman, Pete Zaitcev, linux-usb, Dan Carpenter, stable

Johan/Greg,

Since CUPS uses libusb to communicate with printers these days (well, for over a decade now) so that printing and scanning can coexist, the usblp driver really doesn't get any usage anymore.


> On Jan 4, 2021, at 10:11 AM, Johan Hovold <johan@kernel.org> wrote:
> 
> On Mon, Jan 04, 2021 at 03:59:02PM +0100, Greg Kroah-Hartman wrote:
>> On Mon, Jan 04, 2021 at 03:53:02PM +0100, Johan Hovold wrote:
>>> Stack-allocated buffers cannot be used for DMA (on all architectures).
>>> 
>>> Replace the HP-channel macro with a helper function that allocates a
>>> dedicated transfer buffer so that it can continue to be used with
>>> arguments from the stack.
> 
>> Wow, no one uses this driver anymore it seems, this should have
>> triggered a runtime warning on newer kernels :(
> 
> This helper is only used by the IOCNR_HP_SET_CHANNEL ioctl so perhaps
> it's just that one which isn't used much.
> 
> Johan
> 

________________________
Michael Sweet




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] USB: usblp: fix DMA to stack
  2021-01-04 15:33     ` Michael Sweet
@ 2021-01-04 15:48       ` Johan Hovold
  0 siblings, 0 replies; 7+ messages in thread
From: Johan Hovold @ 2021-01-04 15:48 UTC (permalink / raw)
  To: Michael Sweet
  Cc: Johan Hovold, Greg Kroah-Hartman, Pete Zaitcev, linux-usb,
	Dan Carpenter, stable

On Mon, Jan 04, 2021 at 10:33:34AM -0500, Michael Sweet wrote:
> Johan/Greg,
> 
> Since CUPS uses libusb to communicate with printers these days (well,
> for over a decade now) so that printing and scanning can coexist, the
> usblp driver really doesn't get any usage anymore.

Ah, ok. I still seem to be using usblp.ko for my printer with CUPS,
though.

Johan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] USB: usblp: fix DMA to stack
  2021-01-04 14:53 [PATCH] USB: usblp: fix DMA to stack Johan Hovold
  2021-01-04 14:59 ` Greg Kroah-Hartman
@ 2021-01-04 17:37 ` Pete Zaitcev
  2021-01-06 11:25   ` Johan Hovold
  1 sibling, 1 reply; 7+ messages in thread
From: Pete Zaitcev @ 2021-01-04 17:37 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Greg Kroah-Hartman, linux-usb, Dan Carpenter, stable, zaitcev

On Mon,  4 Jan 2021 15:53:02 +0100
Johan Hovold <johan@kernel.org> wrote:

> +++ b/drivers/usb/class/usblp.c
> -#define usblp_hp_channel_change_request(usblp, channel, buffer) \
> -	usblp_ctrl_msg(usblp, USBLP_REQ_HP_CHANNEL_CHANGE_REQUEST, USB_TYPE_VENDOR, USB_DIR_IN, USB_RECIP_INTERFACE, channel, buffer, 1)
> +static int usblp_hp_channel_change_request(struct usblp *usblp, int channel, u8 *new_channel)

Acked-By: Pete Zaitcev <zaitcev@redhat.com>

I would probably get rid of the buffer pointer and return
new_channel & 0xFF in case of success. That would kill
the newChannel too, and there's no need to debage u8 versus
unsigned char. But this is good enough. A function is better
than trying to cram the kfree() into the clause of the switch.

-- Pete


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] USB: usblp: fix DMA to stack
  2021-01-04 17:37 ` Pete Zaitcev
@ 2021-01-06 11:25   ` Johan Hovold
  0 siblings, 0 replies; 7+ messages in thread
From: Johan Hovold @ 2021-01-06 11:25 UTC (permalink / raw)
  To: Pete Zaitcev
  Cc: Johan Hovold, Greg Kroah-Hartman, linux-usb, Dan Carpenter, stable

On Mon, Jan 04, 2021 at 11:37:36AM -0600, Pete Zaitcev wrote:
> On Mon,  4 Jan 2021 15:53:02 +0100
> Johan Hovold <johan@kernel.org> wrote:
> 
> > +++ b/drivers/usb/class/usblp.c
> > -#define usblp_hp_channel_change_request(usblp, channel, buffer) \
> > -	usblp_ctrl_msg(usblp, USBLP_REQ_HP_CHANNEL_CHANGE_REQUEST, USB_TYPE_VENDOR, USB_DIR_IN, USB_RECIP_INTERFACE, channel, buffer, 1)
> > +static int usblp_hp_channel_change_request(struct usblp *usblp, int channel, u8 *new_channel)
> 
> Acked-By: Pete Zaitcev <zaitcev@redhat.com>
> 
> I would probably get rid of the buffer pointer and return
> new_channel & 0xFF in case of success. That would kill
> the newChannel too, and there's no need to debage u8 versus
> unsigned char. But this is good enough. A function is better
> than trying to cram the kfree() into the clause of the switch.

Yeah, I wanted a minimal change suitable for stable and the helper was
already there to be used for this.

Johan

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-01-06 11:26 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-04 14:53 [PATCH] USB: usblp: fix DMA to stack Johan Hovold
2021-01-04 14:59 ` Greg Kroah-Hartman
2021-01-04 15:11   ` Johan Hovold
2021-01-04 15:33     ` Michael Sweet
2021-01-04 15:48       ` Johan Hovold
2021-01-04 17:37 ` Pete Zaitcev
2021-01-06 11:25   ` Johan Hovold

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.