Yes Dan, I should have done better. Also Tom suggested to get rid of NULL pointer check as its redundant because callee does that before calling __free_irq.
On Tue, Jul 17, 2018 at 03:25:57PM +0300, Dan Carpenter wrote: > Hi RAGHU, > > Thank you for the patch! Perhaps something to improve: > > url: > https://github.com/0day-ci/linux/commits/RAGHU-Halharvi/genirq-Dereference-desc-after-null-pointer-check/20180717-150842 > > smatch warnings: > kernel/irq/manage.c:1571 __free_irq() error: uninitialized symbol 'irq'. > > # > https://github.com/0day-ci/linux/commit/3ebb02d34bcb1c8111a79856326bca85fb3321eb > git remote add linux-review https://github.com/0day-ci/linux > git remote update linux-review > git checkout 3ebb02d34bcb1c8111a79856326bca85fb3321eb > vim +/irq +1571 kernel/irq/manage.c > > d3c60047 Thomas Gleixner 2008-10-16 1560 > cbf94f06 Magnus Damm 2009-03-12 1561 /* > cbf94f06 Magnus Damm 2009-03-12 1562 * Internal > function to unregister an irqaction - used to free > cbf94f06 Magnus Damm 2009-03-12 1563 * regular and > special interrupts that are part of the architecture. > ^1da177e Linus Torvalds 2005-04-16 1564 */ > 83ac4ca9 Uwe Kleine König 2018-03-19 1565 static struct > irqaction *__free_irq(struct irq_desc *desc, void *dev_id) > ^1da177e Linus Torvalds 2005-04-16 1566 { > 3ebb02d3 RAGHU Halharvi 2018-07-17 1567 unsigned int > irq; > f17c7545 Ingo Molnar 2009-02-17 1568 struct > irqaction *action, **action_ptr; > ^1da177e Linus Torvalds 2005-04-16 1569 unsigned long > flags; > ^1da177e Linus Torvalds 2005-04-16 1570 > ae88a23b Ingo Molnar 2009-02-15 @1571 > WARN(in_interrupt(), "Trying to free IRQ %d from IRQ context!\n", irq); > > ^^^ > Not initialized. > > 7d94f7ca Yinghai Lu 2008-08-19 1572 > 7d94f7ca Yinghai Lu 2008-08-19 1573 if (!desc) > f21cfb25 Magnus Damm 2009-03-12 1574 return > NULL; > ^1da177e Linus Torvalds 2005-04-16 1575 > 3ebb02d3 RAGHU Halharvi 2018-07-17 1576 irq = > desc->irq_data.irq; > 9114014c Thomas Gleixner 2017-06-29 1577 > mutex_lock(&desc->request_mutex); > abc7e40c Thomas Gleixner 2015-12-13 1578 > chip_bus_lock(desc); > 239007b8 Thomas Gleixner 2009-11-17 1579 > raw_spin_lock_irqsave(&desc->lock, flags); > ae88a23b Ingo Molnar 2009-02-15 1580 > ae88a23b Ingo Molnar 2009-02-15 1581 /* > ae88a23b Ingo Molnar 2009-02-15 1582 * There can be > multiple actions per IRQ descriptor, find the right > ae88a23b Ingo Molnar 2009-02-15 1583 * one based on > the dev_id: > ae88a23b Ingo Molnar 2009-02-15 1584 */ > f17c7545 Ingo Molnar 2009-02-17 1585 action_ptr = > &desc->action; > ^1da177e Linus Torvalds 2005-04-16 1586 for (;;) { > f17c7545 Ingo Molnar 2009-02-17 1587 action > = *action_ptr; > ^1da177e Linus Torvalds 2005-04-16 1588 > ae88a23b Ingo Molnar 2009-02-15 1589 if > (!action) { > ae88a23b Ingo Molnar 2009-02-15 1590 > WARN(1, "Trying to free already-free IRQ %d\n", irq); > 239007b8 Thomas Gleixner 2009-11-17 1591 > raw_spin_unlock_irqrestore(&desc->lock, flags); > abc7e40c Thomas Gleixner 2015-12-13 1592 > chip_bus_sync_unlock(desc); > 19d39a38 Thomas Gleixner 2017-07-11 1593 > mutex_unlock(&desc->request_mutex); > f21cfb25 Magnus Damm 2009-03-12 1594 > return NULL; > ae88a23b Ingo Molnar 2009-02-15 1595 } > ^1da177e Linus Torvalds 2005-04-16 1596 > 8316e381 Ingo Molnar 2009-02-17 1597 if > (action->dev_id == dev_id) > ae88a23b Ingo Molnar 2009-02-15 1598 > break; > f17c7545 Ingo Molnar 2009-02-17 1599 > action_ptr = &action->next; > ae88a23b Ingo Molnar 2009-02-15 1600 } > ^1da177e Linus Torvalds 2005-04-16 1601 > ae88a23b Ingo Molnar 2009-02-15 1602 /* Found it - > now remove it from the list of entries: */ > f17c7545 Ingo Molnar 2009-02-17 1603 *action_ptr = > action->next; > dbce706e Paolo 'Blaisorblade' Giarrusso 2005-06-21 1604 > cab303be Thomas Gleixner 2014-08-28 1605 > irq_pm_remove_action(desc, action); > cab303be Thomas Gleixner 2014-08-28 1606 > ae88a23b Ingo Molnar 2009-02-15 1607 /* If this was > the last handler, shut down the IRQ line: */ > c1bacbae Thomas Gleixner 2014-03-08 1608 if > (!desc->action) { > e9849777 Thomas Gleixner 2015-10-09 1609 > irq_settings_clr_disable_unlazy(desc); > 46999238 Thomas Gleixner 2011-02-02 1610 > irq_shutdown(desc); > c1bacbae Thomas Gleixner 2014-03-08 1611 } > 3aa551c9 Thomas Gleixner 2009-03-23 1612 > > :::::: The code at line 1571 was first introduced by commit > :::::: ae88a23b32fa7e0dc9fa7ce735966e68eb41b0bc irq: refactor and clean up > the free_irq() code flow > > :::::: TO: Ingo Molnar <[email protected]> > :::::: CC: Ingo Molnar <[email protected]> > > --- > 0-DAY kernel test infrastructure Open Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation
