| Message ID | 20180612015301.27371-1-peter.chen@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jun 12, 2018 at 09:53:01AM +0800, Peter Chen wrote: > The port->logbuffer_head may be wrong if the two processes enters > _tcpm_log at the mostly same time. The 2nd process enters _tcpm_log > before the 1st process update the index, then the 2nd process will > not allocate logbuffer, when the 2nd process tries to use log buffer, > the index has already updated by the 1st process, so it will get > NULL pointer for updated logbuffer, the error message like below: > > tcpci 0-0050: Log buffer index 6 is NULL > > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Cc: Guenter Roeck <linux@roeck-us.net> > Cc: Jun Li <jun.li@nxp.com> > Signed-off-by: Peter Chen <peter.chen@nxp.com> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > drivers/usb/typec/tcpm.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c > index 0ccd2ce1eb59..2ed07d0bf364 100644 > --- a/drivers/usb/typec/tcpm.c > +++ b/drivers/usb/typec/tcpm.c > @@ -418,17 +418,18 @@ static void _tcpm_log(struct tcpm_port *port, const char *fmt, va_list args) > u64 ts_nsec = local_clock(); > unsigned long rem_nsec; > > + mutex_lock(&port->logbuffer_lock); > if (!port->logbuffer[port->logbuffer_head]) { > port->logbuffer[port->logbuffer_head] = > kzalloc(LOG_BUFFER_ENTRY_SIZE, GFP_KERNEL); > - if (!port->logbuffer[port->logbuffer_head]) > + if (!port->logbuffer[port->logbuffer_head]) { > + mutex_unlock(&port->logbuffer_lock); > return; > + } > } > > vsnprintf(tmpbuffer, sizeof(tmpbuffer), fmt, args); > > - mutex_lock(&port->logbuffer_lock); > - > if (tcpm_log_full(port)) { > port->logbuffer_head = max(port->logbuffer_head - 1, 0); > strcpy(tmpbuffer, "overflow"); Thanks,
On 06/11/2018 06:53 PM, Peter Chen wrote: > The port->logbuffer_head may be wrong if the two processes enters > _tcpm_log at the mostly same time. The 2nd process enters _tcpm_log > before the 1st process update the index, then the 2nd process will > not allocate logbuffer, when the 2nd process tries to use log buffer, > the index has already updated by the 1st process, so it will get > NULL pointer for updated logbuffer, the error message like below: > > tcpci 0-0050: Log buffer index 6 is NULL > > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Cc: Guenter Roeck <linux@roeck-us.net> > Cc: Jun Li <jun.li@nxp.com> > Signed-off-by: Peter Chen <peter.chen@nxp.com> Reviewed-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/usb/typec/tcpm.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c > index 0ccd2ce1eb59..2ed07d0bf364 100644 > --- a/drivers/usb/typec/tcpm.c > +++ b/drivers/usb/typec/tcpm.c > @@ -418,17 +418,18 @@ static void _tcpm_log(struct tcpm_port *port, const char *fmt, va_list args) > u64 ts_nsec = local_clock(); > unsigned long rem_nsec; > > + mutex_lock(&port->logbuffer_lock); > if (!port->logbuffer[port->logbuffer_head]) { > port->logbuffer[port->logbuffer_head] = > kzalloc(LOG_BUFFER_ENTRY_SIZE, GFP_KERNEL); > - if (!port->logbuffer[port->logbuffer_head]) > + if (!port->logbuffer[port->logbuffer_head]) { > + mutex_unlock(&port->logbuffer_lock); > return; > + } > } > > vsnprintf(tmpbuffer, sizeof(tmpbuffer), fmt, args); > > - mutex_lock(&port->logbuffer_lock); > - > if (tcpm_log_full(port)) { > port->logbuffer_head = max(port->logbuffer_head - 1, 0); > strcpy(tmpbuffer, "overflow"); > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c index 0ccd2ce1eb59..2ed07d0bf364 100644 --- a/drivers/usb/typec/tcpm.c +++ b/drivers/usb/typec/tcpm.c @@ -418,17 +418,18 @@ static void _tcpm_log(struct tcpm_port *port, const char *fmt, va_list args) u64 ts_nsec = local_clock(); unsigned long rem_nsec; + mutex_lock(&port->logbuffer_lock); if (!port->logbuffer[port->logbuffer_head]) { port->logbuffer[port->logbuffer_head] = kzalloc(LOG_BUFFER_ENTRY_SIZE, GFP_KERNEL); - if (!port->logbuffer[port->logbuffer_head]) + if (!port->logbuffer[port->logbuffer_head]) { + mutex_unlock(&port->logbuffer_lock); return; + } } vsnprintf(tmpbuffer, sizeof(tmpbuffer), fmt, args); - mutex_lock(&port->logbuffer_lock); - if (tcpm_log_full(port)) { port->logbuffer_head = max(port->logbuffer_head - 1, 0); strcpy(tmpbuffer, "overflow");
The port->logbuffer_head may be wrong if the two processes enters _tcpm_log at the mostly same time. The 2nd process enters _tcpm_log before the 1st process update the index, then the 2nd process will not allocate logbuffer, when the 2nd process tries to use log buffer, the index has already updated by the 1st process, so it will get NULL pointer for updated logbuffer, the error message like below: tcpci 0-0050: Log buffer index 6 is NULL Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> Cc: Guenter Roeck <linux@roeck-us.net> Cc: Jun Li <jun.li@nxp.com> Signed-off-by: Peter Chen <peter.chen@nxp.com> --- drivers/usb/typec/tcpm.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)