Re: [PATCH v5 2/6] usb: gadget: u_serial: reimplement console support
On Fri, Aug 09, 2019 at 03:05:55PM +0300, Felipe Balbi wrote: > > Hi, > > Michał Mirosław writes: > > Rewrite console support to fix a few shortcomings of the old code > > preventing its use with multiple ports. This removes some duplicated > > code and replaces a custom kthread with simpler workqueue item. > > > > Only port ttyGS0 gets to be a console for now. > > > > Signed-off-by: Michał Mirosław > > Reviewed-by: Greg Kroah-Hartman > > Tested-by: Ladislav Michl > > checking file drivers/usb/gadget/function/u_serial.c > Hunk #7 FAILED at 1206. > Hunk #8 succeeded at 1302 (offset -2 lines). > Hunk #9 succeeded at 1334 (offset -2 lines). > Hunk #10 succeeded at 1363 (offset -2 lines). > 1 out of 10 hunks FAILED > > Could you rebase on my testing/next? Sure. Should be ready in a few minutes. Best Regards, Michał Mirosław
[PATCH v6 4/7] usb: gadget: u_serial: allow more console gadget ports
Allow configuring more than one console using USB serial or ACM gadget.
By default, only first (ttyGS0) is a console, but this may be changed
using function's new "console" attribute.
Signed-off-by: Michał Mirosław
---
v6: rebased on balbi/testing/next
v5: fixed locking in gserial_get_console()
v4: fixed locking in gserial_set_console()
v3: no changes
v2: no changes
---
drivers/usb/gadget/function/f_acm.c| 21 +++
drivers/usb/gadget/function/f_serial.c | 21 +++
drivers/usb/gadget/function/u_serial.c | 48 ++
drivers/usb/gadget/function/u_serial.h | 7
4 files changed, 97 insertions(+)
diff --git a/drivers/usb/gadget/function/f_acm.c
b/drivers/usb/gadget/function/f_acm.c
index 9fc98de83624..7c152c28b26c 100644
--- a/drivers/usb/gadget/function/f_acm.c
+++ b/drivers/usb/gadget/function/f_acm.c
@@ -771,6 +771,24 @@ static struct configfs_item_operations acm_item_ops = {
.release= acm_attr_release,
};
+#ifdef CONFIG_U_SERIAL_CONSOLE
+
+static ssize_t f_acm_console_store(struct config_item *item,
+ const char *page, size_t count)
+{
+ return gserial_set_console(to_f_serial_opts(item)->port_num,
+ page, count);
+}
+
+static ssize_t f_acm_console_show(struct config_item *item, char *page)
+{
+ return gserial_get_console(to_f_serial_opts(item)->port_num, page);
+}
+
+CONFIGFS_ATTR(f_acm_, console);
+
+#endif /* CONFIG_U_SERIAL_CONSOLE */
+
static ssize_t f_acm_port_num_show(struct config_item *item, char *page)
{
return sprintf(page, "%u\n", to_f_serial_opts(item)->port_num);
@@ -779,6 +797,9 @@ static ssize_t f_acm_port_num_show(struct config_item
*item, char *page)
CONFIGFS_ATTR_RO(f_acm_, port_num);
static struct configfs_attribute *acm_attrs[] = {
+#ifdef CONFIG_U_SERIAL_CONSOLE
+ &f_acm_attr_console,
+#endif
&f_acm_attr_port_num,
NULL,
};
diff --git a/drivers/usb/gadget/function/f_serial.c
b/drivers/usb/gadget/function/f_serial.c
index c860f30a0ea2..1406255d0865 100644
--- a/drivers/usb/gadget/function/f_serial.c
+++ b/drivers/usb/gadget/function/f_serial.c
@@ -266,6 +266,24 @@ static struct configfs_item_operations serial_item_ops = {
.release= serial_attr_release,
};
+#ifdef CONFIG_U_SERIAL_CONSOLE
+
+static ssize_t f_serial_console_store(struct config_item *item,
+ const char *page, size_t count)
+{
+ return gserial_set_console(to_f_serial_opts(item)->port_num,
+ page, count);
+}
+
+static ssize_t f_serial_console_show(struct config_item *item, char *page)
+{
+ return gserial_get_console(to_f_serial_opts(item)->port_num, page);
+}
+
+CONFIGFS_ATTR(f_serial_, console);
+
+#endif /* CONFIG_U_SERIAL_CONSOLE */
+
static ssize_t f_serial_port_num_show(struct config_item *item, char *page)
{
return sprintf(page, "%u\n", to_f_serial_opts(item)->port_num);
@@ -274,6 +292,9 @@ static ssize_t f_serial_port_num_show(struct config_item
*item, char *page)
CONFIGFS_ATTR_RO(f_serial_, port_num);
static struct configfs_attribute *acm_attrs[] = {
+#ifdef CONFIG_U_SERIAL_CONSOLE
+ &f_serial_attr_console,
+#endif
&f_serial_attr_port_num,
NULL,
};
diff --git a/drivers/usb/gadget/function/u_serial.c
b/drivers/usb/gadget/function/u_serial.c
index 62280c23cde2..0da00546006f 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1081,6 +1081,54 @@ static void gs_console_exit(struct gs_port *port)
port->console = NULL;
}
+ssize_t gserial_set_console(unsigned char port_num, const char *page, size_t
count)
+{
+ struct gs_port *port;
+ bool enable;
+ int ret;
+
+ ret = strtobool(page, &enable);
+ if (ret)
+ return ret;
+
+ mutex_lock(&ports[port_num].lock);
+ port = ports[port_num].port;
+
+ if (WARN_ON(port == NULL)) {
+ ret = -ENXIO;
+ goto out;
+ }
+
+ if (enable)
+ ret = gs_console_init(port);
+ else
+ gs_console_exit(port);
+out:
+ mutex_unlock(&ports[port_num].lock);
+
+ return ret < 0 ? ret : count;
+}
+EXPORT_SYMBOL_GPL(gserial_set_console);
+
+ssize_t gserial_get_console(unsigned char port_num, char *page)
+{
+ struct gs_port *port;
+ ssize_t ret;
+
+ mutex_lock(&ports[port_num].lock);
+ port = ports[port_num].port;
+
+ if (WARN_ON(port == NULL))
+ ret = -ENXIO;
+ else
+ ret = sprintf(page, "%u\n", !!port->console);
+
+ mutex_unlock(&ports[port_num].lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(gserial_get_console);
+
#else
static int gs_console_connect(struct gs_port *port)
diff --git a/drivers/usb/gadget/function/u_serial.h
b/drivers/usb/gadget/function/u_serial.h
index 8b472b0c8cb4..e5b08ab8cf7a 100644
--- a/drivers/usb/gadget/function/
[PATCH v6 3/7] usb: gadget: u_serial: make OBEX port not a console
Prevent OBEX serial port from ever becoming a console. Console messages
will definitely break the protocol, and since you have to instantiate
the port making it explicitly for OBEX, there is no point in allowing
console to break it by mistake.
Signed-off-by: Michał Mirosław
Reviewed-by: Greg Kroah-Hartman
---
v6: rebased on balbi/testing/next
v5: no changes
v4: no changes
v3: rename gserial_alloc_line_raw() -> gserial_alloc_line_no_console()
v2: change of API
+ commit message massage
---
drivers/usb/gadget/function/f_obex.c | 2 +-
drivers/usb/gadget/function/u_serial.c | 16
drivers/usb/gadget/function/u_serial.h | 1 +
3 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/function/f_obex.c
b/drivers/usb/gadget/function/f_obex.c
index 55b7f57d2dc7..ab26d84ed95e 100644
--- a/drivers/usb/gadget/function/f_obex.c
+++ b/drivers/usb/gadget/function/f_obex.c
@@ -432,7 +432,7 @@ static struct usb_function_instance *obex_alloc_inst(void)
return ERR_PTR(-ENOMEM);
opts->func_inst.free_func_inst = obex_free_inst;
- ret = gserial_alloc_line(&opts->port_num);
+ ret = gserial_alloc_line_no_console(&opts->port_num);
if (ret) {
kfree(opts);
return ERR_PTR(ret);
diff --git a/drivers/usb/gadget/function/u_serial.c
b/drivers/usb/gadget/function/u_serial.c
index 94f6999e8262..62280c23cde2 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -1180,7 +1180,7 @@ void gserial_free_line(unsigned char port_num)
}
EXPORT_SYMBOL_GPL(gserial_free_line);
-int gserial_alloc_line(unsigned char *line_num)
+int gserial_alloc_line_no_console(unsigned char *line_num)
{
struct usb_cdc_line_coding coding;
struct gs_port *port;
@@ -1221,12 +1221,20 @@ int gserial_alloc_line(unsigned char *line_num)
goto err;
}
*line_num = port_num;
-
- if (!port_num)
- gs_console_init(port);
err:
return ret;
}
+EXPORT_SYMBOL_GPL(gserial_alloc_line_no_console);
+
+int gserial_alloc_line(unsigned char *line_num)
+{
+ int ret = gserial_alloc_line_no_console(line_num);
+
+ if (!ret && !*line_num)
+ gs_console_init(ports[*line_num].port);
+
+ return ret;
+}
EXPORT_SYMBOL_GPL(gserial_alloc_line);
/**
diff --git a/drivers/usb/gadget/function/u_serial.h
b/drivers/usb/gadget/function/u_serial.h
index 9acaac1cbb75..8b472b0c8cb4 100644
--- a/drivers/usb/gadget/function/u_serial.h
+++ b/drivers/usb/gadget/function/u_serial.h
@@ -54,6 +54,7 @@ struct usb_request *gs_alloc_req(struct usb_ep *ep, unsigned
len, gfp_t flags);
void gs_free_req(struct usb_ep *, struct usb_request *req);
/* management of individual TTY ports */
+int gserial_alloc_line_no_console(unsigned char *port_line);
int gserial_alloc_line(unsigned char *port_line);
void gserial_free_line(unsigned char port_line);
--
2.20.1
[PATCH v6 6/7] usb: gadget: legacy/serial: allow dynamic removal
Legacy serial USB gadget is still useful as an early console,
before userspace is up. Later it could be replaced with proper
configfs-configured composite gadget - that use case is enabled
by this patch.
Signed-off-by: Michał Mirosław
---
v6: rebased on balbi/testing/next
v5: no changes
v4: initial revision, new in the patchset
---
drivers/usb/gadget/legacy/serial.c | 49 +-
1 file changed, 48 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/legacy/serial.c
b/drivers/usb/gadget/legacy/serial.c
index de30d7628eef..da44f89f5e73 100644
--- a/drivers/usb/gadget/legacy/serial.c
+++ b/drivers/usb/gadget/legacy/serial.c
@@ -97,6 +97,36 @@ static unsigned n_ports = 1;
module_param(n_ports, uint, 0);
MODULE_PARM_DESC(n_ports, "number of ports to create, default=1");
+static bool enable = true;
+
+static int switch_gserial_enable(bool do_enable);
+
+static int enable_set(const char *s, const struct kernel_param *kp)
+{
+ bool do_enable;
+ int ret;
+
+ if (!s) /* called for no-arg enable == default */
+ return 0;
+
+ ret = strtobool(s, &do_enable);
+ if (ret || enable == do_enable)
+ return ret;
+
+ ret = switch_gserial_enable(do_enable);
+ if (!ret)
+ enable = do_enable;
+
+ return ret;
+}
+
+static const struct kernel_param_ops enable_ops = {
+ .set = enable_set,
+ .get = param_get_bool,
+};
+
+module_param_cb(enable, &enable_ops, &enable, 0644);
+
/*-*/
static struct usb_configuration serial_config_driver = {
@@ -240,6 +270,19 @@ static struct usb_composite_driver gserial_driver = {
.unbind = gs_unbind,
};
+static int switch_gserial_enable(bool do_enable)
+{
+ if (!serial_config_driver.label)
+ /* init() was not called, yet */
+ return 0;
+
+ if (do_enable)
+ return usb_composite_probe(&gserial_driver);
+
+ usb_composite_unregister(&gserial_driver);
+ return 0;
+}
+
static int __init init(void)
{
/* We *could* export two configs; that'd be much cleaner...
@@ -266,12 +309,16 @@ static int __init init(void)
}
strings_dev[STRING_DESCRIPTION_IDX].s = serial_config_driver.label;
+ if (!enable)
+ return 0;
+
return usb_composite_probe(&gserial_driver);
}
module_init(init);
static void __exit cleanup(void)
{
- usb_composite_unregister(&gserial_driver);
+ if (enable)
+ usb_composite_unregister(&gserial_driver);
}
module_exit(cleanup);
--
2.20.1
[PATCH v6 5/7] usb: gadget: u_serial: diagnose missed console messages
Insert markers in console stream marking places where data
is missing. This makes the hole in the data stand out clearly
instead of glueing together unrelated messages.
Example output as seen from USB host side:
[0.064078] pinctrl core: registered pin 16 (UART3_RTS_N PC0) on
7868.pinmux
[0.064130] pinctrl
[missed 114987 bytes]
[4.302299] udevd[134]: starting version 3.2.5
[4.306845] random: udevd: uninitialized urandom read (16 bytes read)
Signed-off-by: Michał Mirosław
Reviewed-by: Greg Kroah-Hartman
---
v6: rebased on balbi/testing/next
v5: no changes
v4: no changes
v3: added example output
+ lowercase "missed"
v2: commit message massage
---
drivers/usb/gadget/function/u_serial.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/function/u_serial.c
b/drivers/usb/gadget/function/u_serial.c
index 0da00546006f..a248ed0fd5d2 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -88,6 +88,7 @@ struct gs_console {
spinlock_t lock;
struct usb_request *req;
struct kfifobuf;
+ size_t missed;
};
/*
@@ -931,6 +932,15 @@ static void __gs_console_push(struct gs_console *cons)
if (!size)
return;
+ if (cons->missed && ep->maxpacket >= 64) {
+ char buf[64];
+ size_t len;
+
+ len = sprintf(buf, "\n[missed %zu bytes]\n", cons->missed);
+ kfifo_in(&cons->buf, buf, len);
+ cons->missed = 0;
+ }
+
req->length = size;
if (usb_ep_queue(ep, req, GFP_ATOMIC))
req->length = 0;
@@ -952,10 +962,13 @@ static void gs_console_write(struct console *co,
{
struct gs_console *cons = container_of(co, struct gs_console, console);
unsigned long flags;
+ size_t n;
spin_lock_irqsave(&cons->lock, flags);
- kfifo_in(&cons->buf, buf, count);
+ n = kfifo_in(&cons->buf, buf, count);
+ if (n < count)
+ cons->missed += count - n;
if (cons->req && !cons->req->length)
schedule_work(&cons->work);
--
2.20.1
[PATCH v6 2/7] usb: gadget: u_serial: reimplement console support
Rewrite console support to fix a few shortcomings of the old code
preventing its use with multiple ports. This removes some duplicated
code and replaces a custom kthread with simpler workqueue item.
Only port ttyGS0 gets to be a console for now.
Signed-off-by: Michał Mirosław
Reviewed-by: Greg Kroah-Hartman
Tested-by: Ladislav Michl
---
v6: rebased on balbi/testing/next
v5: no changes
v4: cosmetic change to __gs_console_push()
v3: no changes
v2: no changes
---
drivers/usb/gadget/function/u_serial.c | 351 -
1 file changed, 164 insertions(+), 187 deletions(-)
diff --git a/drivers/usb/gadget/function/u_serial.c
b/drivers/usb/gadget/function/u_serial.c
index bb1e2e1d0076..94f6999e8262 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -82,14 +82,12 @@
#define GS_CONSOLE_BUF_SIZE8192
/* console info */
-struct gscons_info {
- struct gs_port *port;
- struct task_struct *console_thread;
- struct kfifocon_buf;
- /* protect the buf and busy flag */
- spinlock_t con_lock;
- int req_busy;
- struct usb_request *console_req;
+struct gs_console {
+ struct console console;
+ struct work_struct work;
+ spinlock_t lock;
+ struct usb_request *req;
+ struct kfifobuf;
};
/*
@@ -101,6 +99,9 @@ struct gs_port {
spinlock_t port_lock; /* guard port_* access */
struct gserial *port_usb;
+#ifdef CONFIG_U_SERIAL_CONSOLE
+ struct gs_console *console;
+#endif
boolopenclose; /* open/close in progress */
u8 port_num;
@@ -889,36 +890,9 @@ static struct tty_driver *gs_tty_driver;
#ifdef CONFIG_U_SERIAL_CONSOLE
-static struct gscons_info gscons_info;
-static struct console gserial_cons;
-
-static struct usb_request *gs_request_new(struct usb_ep *ep)
+static void gs_console_complete_out(struct usb_ep *ep, struct usb_request *req)
{
- struct usb_request *req = usb_ep_alloc_request(ep, GFP_ATOMIC);
- if (!req)
- return NULL;
-
- req->buf = kmalloc(ep->maxpacket, GFP_ATOMIC);
- if (!req->buf) {
- usb_ep_free_request(ep, req);
- return NULL;
- }
-
- return req;
-}
-
-static void gs_request_free(struct usb_request *req, struct usb_ep *ep)
-{
- if (!req)
- return;
-
- kfree(req->buf);
- usb_ep_free_request(ep, req);
-}
-
-static void gs_complete_out(struct usb_ep *ep, struct usb_request *req)
-{
- struct gscons_info *info = &gscons_info;
+ struct gs_console *cons = req->context;
switch (req->status) {
default:
@@ -927,12 +901,12 @@ static void gs_complete_out(struct usb_ep *ep, struct
usb_request *req)
/* fall through */
case 0:
/* normal completion */
- spin_lock(&info->con_lock);
- info->req_busy = 0;
- spin_unlock(&info->con_lock);
-
- wake_up_process(info->console_thread);
+ spin_lock(&cons->lock);
+ req->length = 0;
+ schedule_work(&cons->work);
+ spin_unlock(&cons->lock);
break;
+ case -ECONNRESET:
case -ESHUTDOWN:
/* disconnect */
pr_vdebug("%s: %s shutdown\n", __func__, ep->name);
@@ -940,190 +914,190 @@ static void gs_complete_out(struct usb_ep *ep, struct
usb_request *req)
}
}
-static int gs_console_connect(int port_num)
+static void __gs_console_push(struct gs_console *cons)
{
- struct gscons_info *info = &gscons_info;
- struct gs_port *port;
+ struct usb_request *req = cons->req;
struct usb_ep *ep;
+ size_t size;
- if (port_num != gserial_cons.index) {
- pr_err("%s: port num [%d] is not support console\n",
- __func__, port_num);
- return -ENXIO;
- }
+ if (!req)
+ return; /* disconnected */
- port = ports[port_num].port;
- ep = port->port_usb->in;
- if (!info->console_req) {
- info->console_req = gs_request_new(ep);
- if (!info->console_req)
- return -ENOMEM;
- info->console_req->complete = gs_complete_out;
- }
+ if (req->length)
+ return; /* busy */
- info->port = port;
- spin_lock(&info->con_lock);
- info->req_busy = 0;
- spin_unlock(&info->con_lock);
- pr_vdebug("port[%d] console connect!\n", port_num);
- return 0;
-}
-
-static void gs_console_disconnect(struct usb_ep *ep)
-{
- struct gscons_info *info = &gscons_info;
- struct usb_request *req = info->console_req;
-
- gs_request_free(req, ep
[PATCH v6 7/7] usb: gadget: u_serial: use mutex for serialising open()s
Remove home-made waiting mechanism from gs_open() and rely on
portmaster's mutex to do the job.
Note: This releases thread waiting on close() when another thread
open()s simultaneously.
Signed-off-by: Michał Mirosław
---
v6: initial revision, new in the patchset
---
drivers/usb/gadget/function/u_serial.c | 112 -
1 file changed, 37 insertions(+), 75 deletions(-)
diff --git a/drivers/usb/gadget/function/u_serial.c
b/drivers/usb/gadget/function/u_serial.c
index a248ed0fd5d2..f986e5c55974 100644
--- a/drivers/usb/gadget/function/u_serial.c
+++ b/drivers/usb/gadget/function/u_serial.c
@@ -104,7 +104,6 @@ struct gs_port {
struct gs_console *console;
#endif
- boolopenclose; /* open/close in progress */
u8 port_num;
struct list_headread_pool;
@@ -588,82 +587,45 @@ static int gs_open(struct tty_struct *tty, struct file
*file)
{
int port_num = tty->index;
struct gs_port *port;
- int status;
+ int status = 0;
- do {
- mutex_lock(&ports[port_num].lock);
- port = ports[port_num].port;
- if (!port)
- status = -ENODEV;
- else {
- spin_lock_irq(&port->port_lock);
+ mutex_lock(&ports[port_num].lock);
+ port = ports[port_num].port;
+ if (!port) {
+ status = -ENODEV;
+ goto out;
+ }
- /* already open? Great. */
- if (port->port.count) {
- status = 0;
- port->port.count++;
-
- /* currently opening/closing? wait ... */
- } else if (port->openclose) {
- status = -EBUSY;
-
- /* ... else we do the work */
- } else {
- status = -EAGAIN;
- port->openclose = true;
- }
- spin_unlock_irq(&port->port_lock);
- }
- mutex_unlock(&ports[port_num].lock);
-
- switch (status) {
- default:
- /* fully handled */
- return status;
- case -EAGAIN:
- /* must do the work */
- break;
- case -EBUSY:
- /* wait for EAGAIN task to finish */
- msleep(1);
- /* REVISIT could have a waitchannel here, if
-* concurrent open performance is important
-*/
- break;
- }
- } while (status != -EAGAIN);
-
- /* Do the "real open" */
spin_lock_irq(&port->port_lock);
/* allocate circular buffer on first open */
if (!kfifo_initialized(&port->port_write_buf)) {
spin_unlock_irq(&port->port_lock);
+
+ /*
+* portmaster's mutex still protects from simultaneous open(),
+* and close() can't happen, yet.
+*/
+
status = kfifo_alloc(&port->port_write_buf,
WRITE_BUF_SIZE, GFP_KERNEL);
- spin_lock_irq(&port->port_lock);
-
if (status) {
pr_debug("gs_open: ttyGS%d (%p,%p) no buffer\n",
- port->port_num, tty, file);
- port->openclose = false;
- goto exit_unlock_port;
+port_num, tty, file);
+ goto out;
}
+
+ spin_lock_irq(&port->port_lock);
}
- /* REVISIT if REMOVED (ports[].port NULL), abort the open
-* to let rmmod work faster (but this way isn't wrong).
-*/
-
- /* REVISIT maybe wait for "carrier detect" */
+ /* already open? Great. */
+ if (port->port.count++)
+ goto exit_unlock_port;
tty->driver_data = port;
port->port.tty = tty;
- port->port.count = 1;
- port->openclose = false;
-
/* if connected, start the I/O stream */
if (port->port_usb) {
struct gserial *gser = port->port_usb;
@@ -677,20 +639,21 @@ static int gs_open(struct tty_struct *tty, struct file
*file)
pr_debug("gs_open: ttyGS%d (%p,%p)\n", port->port_num, tty, file);
- status = 0;
-
exit_unlock_port:
spin_unlock_irq(&port->port_lock);
+out:
+ mutex_unlock(&ports[port_num].lock);
return status;
}
-static int gs_writes_finished(struct gs_port *p)
+static int gs_close_flush_done(struct gs_port *p)
{
int cond;
- /* return true on disconnect or empty buffer */
+ /* re
[PATCH v6 0/7] usb: gadget: u_serial: console improvements
This series makes it possible to have more control over console using USB serial gadget ports. This can be useful when you need more than one USB console or are configuring multiple serial port function via configfs. The patches are against balbi/testing/next tree. You can also pull from: https://rere.qmqm.pl/git/linux usb-console Michał Mirosław (7): usb: gadget: u_serial: add missing port entry locking usb: gadget: u_serial: reimplement console support usb: gadget: u_serial: make OBEX port not a console usb: gadget: u_serial: allow more console gadget ports usb: gadget: u_serial: diagnose missed console messages usb: gadget: legacy/serial: allow dynamic removal usb: gadget: u_serial: use mutex for serialising open()s drivers/usb/gadget/function/f_acm.c| 21 + drivers/usb/gadget/function/f_obex.c | 2 +- drivers/usb/gadget/function/f_serial.c | 21 + drivers/usb/gadget/function/u_serial.c | 532 + drivers/usb/gadget/function/u_serial.h | 8 + drivers/usb/gadget/legacy/serial.c | 49 ++- 6 files changed, 370 insertions(+), 263 deletions(-) -- 2.20.1
[PATCH v6 1/7] usb: gadget: u_serial: add missing port entry locking
gserial_alloc_line() misses locking (for a release barrier) while resetting port entry on TTY allocation failure. Fix this. Cc: sta...@vger.kernel.org Signed-off-by: Michał Mirosław Reviewed-by: Greg Kroah-Hartman Tested-by: Ladislav Michl --- v6: rebased on balbi/testing/next v5: no changes v4: no changes v3: cc-stable v2: no changes --- drivers/usb/gadget/function/u_serial.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/gadget/function/u_serial.c b/drivers/usb/gadget/function/u_serial.c index 65f634ec7fc2..bb1e2e1d0076 100644 --- a/drivers/usb/gadget/function/u_serial.c +++ b/drivers/usb/gadget/function/u_serial.c @@ -1239,8 +1239,10 @@ int gserial_alloc_line(unsigned char *line_num) __func__, port_num, PTR_ERR(tty_dev)); ret = PTR_ERR(tty_dev); + mutex_lock(&ports[port_num].lock); port = ports[port_num].port; ports[port_num].port = NULL; + mutex_unlock(&ports[port_num].lock); gserial_free_port(port); goto err; } -- 2.20.1
Re: [PATCH v2] dt-bindings: usb: renesas_gen3: Rename bindings documentation file to reflect IP block
Hello!
On 10.08.2019 0:37, Simon Horman wrote:
For consistency with the naming of (most) other documentation files for DT
bindings for Renesas IP blocks rename the Renesas USB3.0 peripheral
documentation file from renesas,usb3.txt to renesas,usb3-peri.txt
This refines a recent rename from renesas_usb3.txt to renesas-usb3.txt.
To renesas,usb3.txt, perhaps? That's what I'm seeing as the original file
in this patch...
The motivation is to more accurately reflect the IP block documented in
this file.
Signed-off-by: Simon Horman
Reviewed-by: Geert Uytterhoeven
---
* Based on v5.3-rc1
v2
* Add review tag
* Correct changelog
---
.../devicetree/bindings/usb/{renesas,usb3.txt => renesas,usb3-peri.txt} | 0
1 file changed, 0 insertions(+), 0 deletions(-)
rename Documentation/devicetree/bindings/usb/{renesas,usb3.txt =>
renesas,usb3-peri.txt} (100%)
diff --git a/Documentation/devicetree/bindings/usb/renesas,usb3.txt
b/Documentation/devicetree/bindings/usb/renesas,usb3-peri.txt
similarity index 100%
rename from Documentation/devicetree/bindings/usb/renesas,usb3.txt
rename to Documentation/devicetree/bindings/usb/renesas,usb3-peri.txt
MBR, Sergei
Re: [PATCH 03/22] ARM: omap1: move omap15xx local bus handling to usb.c
Thanks for doing this! The odd platforms have always been very
confusing.
> diff --git a/arch/arm/mach-omap1/include/mach/omap1510.h
> b/arch/arm/mach-omap1/include/mach/omap1510.h
> index 3d235244bf5c..7af9c0c7c5ab 100644
> --- a/arch/arm/mach-omap1/include/mach/omap1510.h
> +++ b/arch/arm/mach-omap1/include/mach/omap1510.h
> @@ -159,4 +159,3 @@
> #define OMAP1510_INT_FPGA23 (OMAP_FPGA_IRQ_BASE + 23)
>
> #endif /* __ASM_ARCH_OMAP15XX_H */
> -
Spurious whitespace change?
> diff --git a/arch/arm/mach-omap1/usb.c b/arch/arm/mach-omap1/usb.c
> index d8e9bbda8f7b..740c876ae46b 100644
> --- a/arch/arm/mach-omap1/usb.c
> +++ b/arch/arm/mach-omap1/usb.c
> @@ -10,6 +10,7 @@
> #include
> #include
> #include
> +#include
>
> #include
>
> @@ -127,6 +128,7 @@ omap_otg_init(struct omap_usb_config *config)
>
> syscon &= ~HST_IDLE_EN;
> ohci_device->dev.platform_data = config;
> +
> status = platform_device_register(ohci_device);
Same here.
> +#define OMAP1510_LB_OFFSET UL(0x3000)
> +#define OMAP1510_LB_DMA_PFN_OFFSET ((OMAP1510_LB_OFFSET - PAGE_OFFSET) >>
> PAGE_SHIFT)
Overly long line.
> +/*
> + * OMAP-1510 specific Local Bus clock on/off
> + */
> +static int omap_1510_local_bus_power(int on)
> +{
> + if (on) {
> + omap_writel((1 << 1) | (1 << 0), OMAP1510_LB_MMU_CTL);
> + udelay(200);
> + } else {
> + omap_writel(0, OMAP1510_LB_MMU_CTL);
> + }
> +
> + return 0;
> +}
The caller never checks the const return value, and on is always true as
well. In fact it seems like omap_1510_local_bus_power and
omap_1510_local_bus_init could probably just be merged into the caller.
> +
> +/*
> + * OMAP-1510 specific Local Bus initialization
> + * NOTE: This assumes 32MB memory size in OMAP1510LB_MEMSIZE.
> + * See also arch/mach-omap/memory.h for __virt_to_dma() and
> + * __dma_to_virt() which need to match with the physical
> + * Local Bus address below.
I think that NOTE is out of date, as __virt_to_dma and __dma_to_virt
don't exist anymore.
> +static int omap_1510_local_bus_init(void)
> +{
> + unsigned int tlb;
> + unsigned long lbaddr, physaddr;
> +
> + omap_writel((omap_readl(OMAP1510_LB_CLOCK_DIV) & 0xfff8) | 0x4,
> +OMAP1510_LB_CLOCK_DIV);
> +
> + /* Configure the Local Bus MMU table */
> + for (tlb = 0; tlb < OMAP1510_LB_MEMSIZE; tlb++) {
> + lbaddr = tlb * 0x0010 + OMAP1510_LB_OFFSET;
> + physaddr = tlb * 0x0010 + PHYS_OFFSET;
> + omap_writel((lbaddr & 0x0fff) >> 22, OMAP1510_LB_MMU_CAM_H);
> + omap_writel(((lbaddr & 0x003ffc00) >> 6) | 0xc,
> +OMAP1510_LB_MMU_CAM_L);
> + omap_writel(physaddr >> 16, OMAP1510_LB_MMU_RAM_H);
> + omap_writel((physaddr & 0xfc00) | 0x300,
> OMAP1510_LB_MMU_RAM_L);
Another > 80 chars line.
> + omap_writel(tlb << 4, OMAP1510_LB_MMU_LCK);
> + omap_writel(0x1, OMAP1510_LB_MMU_LD_TLB);
> + }
> +
> + /* Enable the walking table */
> + omap_writel(omap_readl(OMAP1510_LB_MMU_CTL) | (1 << 3),
> OMAP1510_LB_MMU_CTL);
One more.
> + udelay(200);
> +
> + return 0;
The return value is ignored.
Re: AW: AW: AW: KASAN: use-after-free Read in usbhid_power
Hi, On 09-08-19 14:38, Schmid, Carsten wrote: Hi again, Hey, i did not want to trigger an eartquake in the basement of the kernel ;-) My intention was to prevent some crashes, and help developers to find their bugs. I think my patch exactly does this. Hehe, actually drivers not being able to block unbind has been bugging me for a while now, because there are cases where this would be really helpful. 1) make resources refcounted, have child resources take a ref on the parent 2) Disallow unbind on devices with bound child-devices? Exactly what i was thinking of in first attempts. But i fear that would break even more use cases. Hans, directly regarding the driver: The problem i see is that the xhci_intel_unregister_pdev which is added as an action with devm_add_action_or_reset() is called late by the framework, later than the usb_hcd_pci_remove() in xhci_pci_remove. Is there any chance to trigger this before? This is what Greg meant with "right order". Ah, I missed that part, sure that should be easy, just stop using devm_add_action_or_reset() and do the xhci_intel_unregister_pdev() manually at the right time. The downside of this is that you also need to make sure it happens at the right time from probe error-paths but given the bug you are hitting, I guess that is probably already a problem. @Hans: Sure, will have a look at this. I think i have found where to do that, but need to check how to get the pdev pointer there You probably need to store the pdev pointer in one of the xhci driver's private data structs. Regards, Hans
Re: [PATCH v2] dt-bindings: usb: renesas_gen3: Rename bindings documentation file to reflect IP block
On Sat, Aug 10, 2019 at 08:40:15AM +0200, Geert Uytterhoeven wrote: > Hi Simon, > > On Fri, Aug 9, 2019 at 11:37 PM Simon Horman > wrote: > > For consistency with the naming of (most) other documentation files for DT > > bindings for Renesas IP blocks rename the Renesas USB3.0 peripheral > > documentation file from renesas,usb3.txt to renesas,usb3-peri.txt > > > > This refines a recent rename from renesas_usb3.txt to renesas-usb3.txt. > > s/renesas-usb3.txt/renesas,usb3.txt/ I'll fix it up now, no need for a resend...
[GIT PULL] USB fixes for 5.3-rc4
The following changes since commit 609488bc979f99f805f34e9a32c1e3b71179d10b: Linux 5.3-rc2 (2019-07-28 12:47:02 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git tags/usb-5.3-rc4 for you to fetch changes up to 27709ae4e2fe6cf7da2ae45e718e190c5433342b: usb: setup authorized_default attributes using usb_bus_notify (2019-08-08 16:07:34 +0200) USB fixes for 5.3-rc4 Here are some small USB fixes for 5.3-rc4. The "biggest" one here is moving code from one file to another in order to fix a long-standing race condition with the creation of sysfs files for USB devices. Turns out that there are now userspace tools out there that are hitting this long-known bug, so it's time to fix them. Thankfully the tool-maker in this case fixed the issue :) The other patches in here are all fixes for reported issues. Now that syzbot knows how to fuzz USB drivers better, and is starting to now fuzz the userspace facing side of them at the same time, there will be more and more small fixes like these coming, which is a good thing. All of these have been in linux-next with no reported issues. Signed-off-by: Greg Kroah-Hartman Gavin Li (1): usb: usbfs: fix double-free of usb memory upon submiturb error Guenter Roeck (2): usb: typec: tcpm: Add NULL check before dereferencing config usb: typec: tcpm: Ignore unsupported/unknown alternate mode requests Heikki Krogerus (1): usb: typec: ucsi: ccg: Fix uninitilized symbol error Li Jun (2): usb: typec: tcpm: free log buf memory when remove debug file usb: typec: tcpm: remove tcpm dir if no children Mathias Nyman (1): xhci: Fix NULL pointer dereference at endpoint zero reset. Oliver Neukum (2): Revert "USB: rio500: simplify locking" usb: iowarrior: fix deadlock on disconnect Suzuki K Poulose (1): usb: yurex: Fix use-after-free in yurex_delete Thiébaud Weksteen (1): usb: setup authorized_default attributes using usb_bus_notify Yoshihiro Shimoda (1): usb: host: xhci-rcar: Fix timeout in xhci_suspend() drivers/usb/core/devio.c | 2 - drivers/usb/core/hcd.c| 123 -- drivers/usb/core/sysfs.c | 121 + drivers/usb/core/usb.h| 5 ++ drivers/usb/host/xhci-rcar.c | 9 ++- drivers/usb/host/xhci.c | 10 drivers/usb/misc/iowarrior.c | 7 ++- drivers/usb/misc/rio500.c | 43 - drivers/usb/misc/yurex.c | 2 +- drivers/usb/typec/tcpm/tcpm.c | 58 -- drivers/usb/typec/ucsi/ucsi_ccg.c | 2 +- 11 files changed, 217 insertions(+), 165 deletions(-)
[PATCH] usb: host: fotg2: restart hcd after port reset
From: Hans Ulli Kroll
On the Gemini SoC the FOTG2 stalls after port reset
so restart the HCD after each port reset.
Signed-off-by: Hans Ulli Kroll
Signed-off-by: Linus Walleij
---
drivers/usb/host/fotg210-hcd.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c
index 35f69bce40d9..8ee28f5b7957 100644
--- a/drivers/usb/host/fotg210-hcd.c
+++ b/drivers/usb/host/fotg210-hcd.c
@@ -1633,6 +1633,10 @@ static int fotg210_hub_control(struct usb_hcd *hcd, u16
typeReq, u16 wValue,
/* see what we found out */
temp = check_reset_complete(fotg210, wIndex, status_reg,
fotg210_readl(fotg210, status_reg));
+
+ /* restart schedule */
+ fotg210->command |= CMD_RUN;
+ fotg210_writel(fotg210, fotg210->command,
&fotg210->regs->command);
}
if (!(temp & (PORT_RESUME|PORT_RESET))) {
--
2.21.0
[PATCH] usb: chipidea: imx: fix EPROBE_DEFER support during driver probe
If driver probe needs to be deferred, e.g. because ci_hdrc_add_device()
isn't ready yet, this driver currently misbehaves badly:
a) success is still reported to the driver core (meaning a 2nd
probe attempt will never be done), leaving the driver in
a dysfunctional state and the hardware unusable
b) driver remove / shutdown OOPSes:
[ 206.786916] Unable to handle kernel paging request at virtual address
fdff
[ 206.794148] pgd = 880b9f82
[ 206.796890] [fdff] *pgd=abf5e861, *pte=, *ppte=
[ 206.803179] Internal error: Oops: 37 [#1] PREEMPT SMP ARM
[ 206.808581] Modules linked in: wl18xx evbug
[ 206.813308] CPU: 1 PID: 1 Comm: systemd-shutdow Not tainted
4.19.35+gf345c93b4195 #1
[ 206.821053] Hardware name: Freescale i.MX7 Dual (Device Tree)
[ 206.826813] PC is at ci_hdrc_remove_device+0x4/0x20
[ 206.831699] LR is at ci_hdrc_imx_remove+0x20/0xe8
[ 206.836407] pc : [<805cd4b0>]lr : [<805d62cc>]psr: 2013
[ 206.842678] sp : a806be40 ip : 0001 fp : 80adbd3c
[ 206.847906] r10: 80b1b794 r9 : 80d5dfe0 r8 : a8192c44
[ 206.853136] r7 : 80db93a0 r6 : a8192c10 r5 : a8192c00 r4 : a93a4a00
[ 206.859668] r3 : r2 : a8192ce4 r1 : r0 : fdfb
[ 206.866201] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment
none
[ 206.873341] Control: 10c5387d Table: a9e0c06a DAC: 0051
[ 206.879092] Process systemd-shutdow (pid: 1, stack limit = 0xb271353c)
[ 206.885624] Stack: (0xa806be40 to 0xa806c000)
[ 206.889992] be40: a93a4a00 805d62cc a8192c1c a8170e10 a8192c10 8049a490
80d04d08
[ 206.898179] be60: 80d0da2c fee1dead a806a000 0058
80148b08
[ 206.906366] be80: 01234567 80148d8c a9858600
80d04d08
[ 206.914553] bea0: a82741e0 a9858600 0024 0002
a9858608 0005
[ 206.922740] bec0: 001e 8022c058 a806bf14 a9858600
a806befc
[ 206.930927] bee0: a806bf78 7ee12c30 8022c18c a806bef8 a806befc
0001
[ 206.939115] bf00: 0024 a806bf14 0005 7ee13b34 7ee12c68
0004 7ee13f20
[ 206.947302] bf20: 0010 7ee12c7c 0005 7ee12d04 000a 76e7dc00
0001 80d0f140
[ 206.955490] bf40: ab637880 a974de40 6013 80d0f140 ab6378a0 80d04d08
a8080470 a9858600
[ 206.963677] bf60: a9858600 8022c24c 80144310
[ 206.971864] bf80: 80101204 80d04d08 80d04d08
0003 0058
[ 206.980051] bfa0: 80101204 80101000 fee1dead 28121969
01234567
[ 206.988237] bfc0: 0003 0058
[ 206.996425] bfe0: 0049ffb0 7ee13d58 0048a84b 76f245a6 6030 fee1dead
[ 207.004622] [<805cd4b0>] (ci_hdrc_remove_device) from [<805d62cc>]
(ci_hdrc_imx_remove+0x20/0xe8)
[ 207.013509] [<805d62cc>] (ci_hdrc_imx_remove) from [<8049a490>]
(device_shutdown+0x16c/0x218)
[ 207.022050] [<8049a490>] (device_shutdown) from [<80148b08>]
(kernel_restart+0xc/0x50)
[ 207.029980] [<80148b08>] (kernel_restart) from [<80148d8c>]
(sys_reboot+0xf4/0x1f0)
[ 207.037648] [<80148d8c>] (sys_reboot) from [<80101000>]
(ret_fast_syscall+0x0/0x54)
[ 207.045308] Exception stack(0xa806bfa8 to 0xa806bff0)
[ 207.050368] bfa0: fee1dead 28121969
01234567
[ 207.058554] bfc0: 0003 0058
[ 207.066737] bfe0: 0049ffb0 7ee13d58 0048a84b 76f245a6
[ 207.071799] Code: eba8 e3a0 e8bd8010 e92d4010 (e5904004)
[ 207.078021] ---[ end trace be47424e3fd46e9f ]---
[ 207.082647] Kernel panic - not syncing: Fatal exception
[ 207.087894] ---[ end Kernel panic - not syncing: Fatal exception ]---
c) the error path in combination with driver removal causes
imbalanced calls to the clk_*() and pm_()* APIs
a) happens because the original intended return value is
overwritten (with 0) by the return code of
regulator_disable() in ci_hdrc_imx_probe()'s error path
b) happens because ci_pdev is -EPROBE_DEFER, which causes
ci_hdrc_remove_device() to OOPS
Fix a) by being more careful in ci_hdrc_imx_probe()'s error
path and not overwriting the real error code
Fix b) by calling the respective cleanup functions during
remove only when needed (when ci_pdev != NULL, i.e. when
everything was initialised correctly). This also has the
side effect of not causing imbalanced clk_*() and pm_*()
API calls as part of the error code path.
Fixes: 7c8e8909417e: ("usb: chipidea: imx: add HSIC support")
Signed-off-by: André Draszik
CC: Peter Chen
CC: Greg Kroah-Hartman
CC: Shawn Guo
CC: Sascha Hauer
CC: Pengutronix Kernel Team
CC: Fabio Esteva
Re: BUG: bad usercopy in ld_usb_read
syzbot has found a reproducer for the following crash on: HEAD commit:e96407b4 usb-fuzzer: main usb gadget fuzzer driver git tree: https://github.com/google/kasan.git usb-fuzzer console output: https://syzkaller.appspot.com/x/log.txt?x=17cf0b1660 kernel config: https://syzkaller.appspot.com/x/.config?x=cfa2c18fb6a8068e dashboard link: https://syzkaller.appspot.com/bug?extid=45b2f40f0778cfa7634e compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=151bab1660 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=148f8cd260 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+45b2f40f0778cfa76...@syzkaller.appspotmail.com ldusb 4-1:0.28: Read buffer overflow, -3222596215958809898 bytes dropped usercopy: Kernel memory exposure attempt detected from process stack (offset 0, size 2147479552)! [ cut here ] kernel BUG at mm/usercopy.c:98! invalid opcode: [#1] SMP KASAN CPU: 1 PID: 2023 Comm: syz-executor861 Not tainted 5.3.0-rc2+ #25 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 RIP: 0010:usercopy_abort+0xb9/0xbb mm/usercopy.c:98 Code: e8 c1 f7 d6 ff 49 89 d9 4d 89 e8 4c 89 e1 41 56 48 89 ee 48 c7 c7 e0 f3 cd 85 ff 74 24 08 41 57 48 8b 54 24 20 e8 15 98 c1 ff <0f> 0b e8 95 f7 d6 ff e8 80 9f fd ff 8b 54 24 04 49 89 d8 4c 89 e1 RSP: 0018:8881cbda7c40 EFLAGS: 00010282 RAX: 0061 RBX: 85cdf100 RCX: RDX: RSI: 8128a0fd RDI: ed10397b4f7a RBP: 85cdf2c0 R08: 0061 R09: fbfff11acda1 R10: fbfff11acda0 R11: 88d66d07 R12: 85cdf4e0 R13: 85cdf100 R14: 7000 R15: 85cdf100 FS: 7f10bb76a700() GS:8881db30() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 7f7135a49000 CR3: 0001d20e8000 CR4: 001406e0 Call Trace: __check_object_size mm/usercopy.c:276 [inline] __check_object_size.cold+0x91/0xba mm/usercopy.c:250 check_object_size include/linux/thread_info.h:119 [inline] check_copy_size include/linux/thread_info.h:150 [inline] copy_to_user include/linux/uaccess.h:151 [inline] ld_usb_read+0x304/0x780 drivers/usb/misc/ldusb.c:495 __vfs_read+0x76/0x100 fs/read_write.c:425 vfs_read+0x1ea/0x430 fs/read_write.c:461 ksys_read+0x1e8/0x250 fs/read_write.c:587 do_syscall_64+0xb7/0x580 arch/x86/entry/common.c:296 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x446e19 Code: e8 ec e7 ff ff 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 3b 07 fc ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:7f10bb769d98 EFLAGS: 0246 ORIG_RAX: RAX: ffda RBX: 006dbc38 RCX: 00446e19 RDX: ffbc RSI: 2040 RDI: 0004 RBP: 006dbc30 R08: R09: R10: 000f R11: 0246 R12: 006dbc3c R13: 0001002402090100 R14: 48c920200f11 R15: 08983baa0112 Modules linked in: ---[ end trace 93f3613883c53c00 ]--- RIP: 0010:usercopy_abort+0xb9/0xbb mm/usercopy.c:98 Code: e8 c1 f7 d6 ff 49 89 d9 4d 89 e8 4c 89 e1 41 56 48 89 ee 48 c7 c7 e0 f3 cd 85 ff 74 24 08 41 57 48 8b 54 24 20 e8 15 98 c1 ff <0f> 0b e8 95 f7 d6 ff e8 80 9f fd ff 8b 54 24 04 49 89 d8 4c 89 e1 RSP: 0018:8881cbda7c40 EFLAGS: 00010282 RAX: 0061 RBX: 85cdf100 RCX: RDX: RSI: 8128a0fd RDI: ed10397b4f7a RBP: 85cdf2c0 R08: 0061 R09: fbfff11acda1 R10: fbfff11acda0 R11: 88d66d07 R12: 85cdf4e0 R13: 85cdf100 R14: 7000 R15: 85cdf100 FS: 7f10bb76a700() GS:8881db30() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 7f7135a49000 CR3: 0001d20e8000 CR4: 001406e0
Re: BUG: bad usercopy in ld_usb_read
On Fri, Aug 09, 2019 at 11:13:00AM -0400, Alan Stern wrote: > In fact, I don't see why any of the computations here should overflow > or wrap around, or even give rise to a negative value. If syzbot had a > reproducer we could get more debugging output -- but it doesn't. Yeah, this is odd. The only thing I could see here with more study was that ring_tail is used/updated outside of the rbsl lock in ld_usb_read(). I couldn't convince myself there wasn't a race against the interrupt, but I also couldn't think of a way it could break... -- Kees Cook
Re: [GIT PULL] USB fixes for 5.3-rc4
The pull request you sent on Sat, 10 Aug 2019 13:51:55 +0200: > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git tags/usb-5.3-rc4 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/1041f5092179ab3dfb8ceb8f1f54bc07d4ba3399 Thank you! -- Deet-doot-dot, I am a bot. https://korg.wiki.kernel.org/userdoc/prtracker
RE: [PATCH v9 5/6] usb:cdns3 Add Cadence USB3 DRD Driver
Hi,
>
>Pawel Laszczak writes:
+static int cdns3_gadget_start(struct cdns3 *cdns)
+{
+ struct cdns3_device *priv_dev;
+ u32 max_speed;
+ int ret;
+
+ priv_dev = kzalloc(sizeof(*priv_dev), GFP_KERNEL);
+ if (!priv_dev)
+ return -ENOMEM;
+
+ cdns->gadget_dev = priv_dev;
+ priv_dev->sysdev = cdns->dev;
+ priv_dev->dev = cdns->dev;
+ priv_dev->regs = cdns->dev_regs;
+
+ device_property_read_u16(priv_dev->dev, "cdns,on-chip-buff-size",
+ &priv_dev->onchip_buffers);
+
+ if (priv_dev->onchip_buffers <= 0) {
+ u32 reg = readl(&priv_dev->regs->usb_cap2);
+
+ priv_dev->onchip_buffers = USB_CAP2_ACTUAL_MEM_SIZE(reg);
+ }
+
+ if (!priv_dev->onchip_buffers)
+ priv_dev->onchip_buffers = 256;
+
+ max_speed = usb_get_maximum_speed(cdns->dev);
+
+ /* Check the maximum_speed parameter */
+ switch (max_speed) {
+ case USB_SPEED_FULL:
+ case USB_SPEED_HIGH:
+ case USB_SPEED_SUPER:
+ break;
+ default:
+ dev_err(cdns->dev, "invalid maximum_speed parameter %d\n",
+ max_speed);
+ /* fall through */
+ case USB_SPEED_UNKNOWN:
+ /* default to superspeed */
+ max_speed = USB_SPEED_SUPER;
+ break;
+ }
+
+ /* fill gadget fields */
+ priv_dev->gadget.max_speed = max_speed;
+ priv_dev->gadget.speed = USB_SPEED_UNKNOWN;
+ priv_dev->gadget.ops = &cdns3_gadget_ops;
+ priv_dev->gadget.name = "usb-ss-gadget";
+ priv_dev->gadget.sg_supported = 1;
+
+ spin_lock_init(&priv_dev->lock);
+ INIT_WORK(&priv_dev->pending_status_wq,
+cdns3_pending_setup_status_handler);
+
+ /* initialize endpoint container */
+ INIT_LIST_HEAD(&priv_dev->gadget.ep_list);
+ INIT_LIST_HEAD(&priv_dev->aligned_buf_list);
+
+ ret = cdns3_init_eps(priv_dev);
+ if (ret) {
+ dev_err(priv_dev->dev, "Failed to create endpoints\n");
+ goto err1;
+ }
+
+ /* allocate memory for setup packet buffer */
+ priv_dev->setup_buf = dma_alloc_coherent(priv_dev->sysdev, 8,
+ &priv_dev->setup_dma, GFP_DMA);
+ if (!priv_dev->setup_buf) {
+ ret = -ENOMEM;
+ goto err2;
+ }
+
+ priv_dev->dev_ver = readl(&priv_dev->regs->usb_cap6);
+
+ dev_dbg(priv_dev->dev, "Device Controller version: %08x\n",
+ readl(&priv_dev->regs->usb_cap6));
+ dev_dbg(priv_dev->dev, "USB Capabilities:: %08x\n",
+ readl(&priv_dev->regs->usb_cap1));
+ dev_dbg(priv_dev->dev, "On-Chip memory cnfiguration: %08x\n",
+ readl(&priv_dev->regs->usb_cap2));
+
+ priv_dev->dev_ver = GET_DEV_BASE_VERSION(priv_dev->dev_ver);
+
+ priv_dev->zlp_buf = kzalloc(CDNS3_EP_ZLP_BUF_SIZE, GFP_KERNEL);
+ if (!priv_dev->zlp_buf) {
+ ret = -ENOMEM;
+ goto err3;
+ }
+
+ /* add USB gadget device */
+ ret = usb_add_gadget_udc(priv_dev->dev, &priv_dev->gadget);
+ if (ret < 0) {
+ dev_err(priv_dev->dev,
+ "Failed to register USB device controller\n");
+ goto err4;
+ }
+
+ return 0;
+err4:
+ kfree(priv_dev->zlp_buf);
+err3:
+ dma_free_coherent(priv_dev->sysdev, 8, priv_dev->setup_buf,
+priv_dev->setup_dma);
+err2:
+ cdns3_free_all_eps(priv_dev);
+err1:
+ cdns->gadget_dev = NULL;
+ return ret;
+}
+
+static int __cdns3_gadget_init(struct cdns3 *cdns)
+{
+ struct cdns3_device *priv_dev;
+ int ret = 0;
+
+ cdns3_drd_switch_gadget(cdns, 1);
+ pm_runtime_get_sync(cdns->dev);
+
+ ret = cdns3_gadget_start(cdns);
+ if (ret)
+ return ret;
+
+ priv_dev = cdns->gadget_dev;
+ ret = devm_request_threaded_irq(cdns->dev, cdns->dev_irq,
+ cdns3_device_irq_handler,
+ cdns3_device_thread_irq_handler,
+ IRQF_SHARED, dev_name(cdns->dev), cdns);
>>>
>>>copied handlers here for commenting. Note that you don't have
>>>IRQF_ONESHOT:
>>
>> I know, I can't use IRQF_ ONESHOT flag in this case. I have implemented
>> some code for masking/unmasking interrupts in cdns3_device_irq_handler.
>>
>> Some priority interrupts should be handled ASAP so I can't blocked interrupt
>> Line.
>
>You're completely missing my comment. Your top half should be as short
>as possile. It should only check if current device generated
>interrupts. If it did, then you should wake the thread
[PATCH] media: em28xx: modules workqueue not inited for 2nd device
syzbot reports an error on flush_request_modules() for the second device. This workqueue was never initialised so simply remove the offending line. usb 1-1: USB disconnect, device number 2 em28xx 1-1:1.153: Disconnecting em28xx #1 [ cut here ] WARNING: CPU: 0 PID: 12 at kernel/workqueue.c:3031 __flush_work.cold+0x2c/0x36 kernel/workqueue.c:3031 Kernel panic - not syncing: panic_on_warn set ... CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.3.0-rc2+ #25 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Workqueue: usb_hub_wq hub_event Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0xca/0x13e lib/dump_stack.c:113 panic+0x2a3/0x6da kernel/panic.c:219 __warn.cold+0x20/0x4a kernel/panic.c:576 report_bug+0x262/0x2a0 lib/bug.c:186 fixup_bug arch/x86/kernel/traps.c:179 [inline] fixup_bug arch/x86/kernel/traps.c:174 [inline] do_error_trap+0x12b/0x1e0 arch/x86/kernel/traps.c:272 do_invalid_op+0x32/0x40 arch/x86/kernel/traps.c:291 invalid_op+0x23/0x30 arch/x86/entry/entry_64.S:1026 RIP: 0010:__flush_work.cold+0x2c/0x36 kernel/workqueue.c:3031 Code: 9a 22 00 48 c7 c7 20 e4 c5 85 e8 d9 3a 0d 00 0f 0b 45 31 e4 e9 98 86 ff ff e8 51 9a 22 00 48 c7 c7 20 e4 c5 85 e8 be 3a 0d 00 <0f> 0b 45 31 e4 e9 7d 86 ff ff e8 36 9a 22 00 48 c7 c7 20 e4 c5 85 RSP: 0018:8881da20f720 EFLAGS: 00010286 RAX: 0024 RBX: dc00 RCX: RDX: RSI: 8128a0fd RDI: ed103b441ed6 RBP: 8881da20f888 R08: 0024 R09: fbfff11acd9a R10: fbfff11acd99 R11: 88d66ccf R12: R13: 0001 R14: 8881c6685df8 R15: 8881d2a85b78 flush_request_modules drivers/media/usb/em28xx/em28xx-cards.c:3325 [inline] em28xx_usb_disconnect.cold+0x280/0x2a6 drivers/media/usb/em28xx/em28xx-cards.c:4023 usb_unbind_interface+0x1bd/0x8a0 drivers/usb/core/driver.c:423 __device_release_driver drivers/base/dd.c:1120 [inline] device_release_driver_internal+0x404/0x4c0 drivers/base/dd.c:1151 bus_remove_device+0x2dc/0x4a0 drivers/base/bus.c:556 device_del+0x420/0xb10 drivers/base/core.c:2288 usb_disable_device+0x211/0x690 drivers/usb/core/message.c:1237 usb_disconnect+0x284/0x8d0 drivers/usb/core/hub.c:2199 hub_port_connect drivers/usb/core/hub.c:4949 [inline] hub_port_connect_change drivers/usb/core/hub.c:5213 [inline] port_event drivers/usb/core/hub.c:5359 [inline] hub_event+0x1454/0x3640 drivers/usb/core/hub.c:5441 process_one_work+0x92b/0x1530 kernel/workqueue.c:2269 process_scheduled_works kernel/workqueue.c:2331 [inline] worker_thread+0x7ab/0xe20 kernel/workqueue.c:2417 kthread+0x318/0x420 kernel/kthread.c:255 ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352 Kernel Offset: disabled Rebooting in 86400 seconds.. Reported-by: syzbot+b7f57261c521087d8...@syzkaller.appspotmail.com Signed-off-by: Sean Young --- drivers/media/usb/em28xx/em28xx-cards.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c index 6e33782c3ca6..5983e72a0622 100644 --- a/drivers/media/usb/em28xx/em28xx-cards.c +++ b/drivers/media/usb/em28xx/em28xx-cards.c @@ -4019,7 +4019,6 @@ static void em28xx_usb_disconnect(struct usb_interface *intf) dev->dev_next->disconnected = 1; dev_info(&dev->intf->dev, "Disconnecting %s\n", dev->dev_next->name); - flush_request_modules(dev->dev_next); } dev->disconnected = 1; -- 2.21.0
