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
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
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
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
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
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
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
linux-stable-mirror@lists.linaro.org
-
Greg Kroah-Hartman -
Johan Hovold -
Michael Sweet -
Pete Zaitcev