On Tue, 2017-08-29 at 17:58 +0100, Colin King wrote: > Currently, the pointer em485 is dereferenced to get p and then later > em485 is checked to see if it is null before calling __start_tx. In > the case where em485 is null, we get a null pointer dereference. Fix > this by moving the deference and the associated spinlock/unlocks on > p to the code block where em485 is known to be not null. > static enum hrtimer_restart serial8250_em485_handle_start_tx(struct > hrtimer *t) > { > struct uart_8250_em485 *em485; > - struct uart_8250_port *p; > unsigned long flags; > em485 = container_of(t, struct uart_8250_em485, > start_tx_timer); > - p = em485->port; > > - spin_lock_irqsave(&p->port.lock, flags); > if (em485 && > em485->active_timer == &em485->start_tx_timer) { > + struct uart_8250_port *p = em485->port; > + > + spin_lock_irqsave(&p->port.lock, flags); Can you describe, please, what on your opinion is protected by this lock? > __start_tx(&p->port); > em485->active_timer = NULL; > + spin_unlock_irqrestore(&p->port.lock, flags); > } > - spin_unlock_irqrestore(&p->port.lock, flags); > return HRTIMER_NORESTART; > } > -- Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html