| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20180717123805.GA25982@raghu-VirtualBox>
Date: Tue, 17 Jul 2018 18:08:05 +0530
From: RAGHU H <raghuhack78@...il.com>
To: Dan Carpenter <dan.carpenter@...cle.com>
Cc: kbuild@...org, kbuild-all@...org, linux-kernel@...r.kernel.org,
tglx@...utronix.de
Subject: Re: [PATCH] genirq:Dereference desc after null pointer check
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 <mingo@...e.hu>
> :::::: CC: Ingo Molnar <mingo@...e.hu>
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
Powered by blists - more mailing lists