[PATCH 3/6] drivers/net: remove null pointer dereference - Kernel

This is a discussion on [PATCH 3/6] drivers/net: remove null pointer dereference - Kernel ; From: Julia Lawall If dev is NULL, it is not possible to access its name field. So I have simply modified the error message to drop the printing of the name field. This problem was found using the following semantic ...

+ Reply to Thread
Results 1 to 7 of 7

Thread: [PATCH 3/6] drivers/net: remove null pointer dereference

  1. [PATCH 3/6] drivers/net: remove null pointer dereference

    From: Julia Lawall

    If dev is NULL, it is not possible to access its name field. So I have
    simply modified the error message to drop the printing of the name field.


    This problem was found using the following semantic match
    (http://www.emn.fr/x-info/coccinelle/)

    //
    @@
    expression E, E1;
    identifier f;
    statement S1,S2,S3;
    @@

    * if (E == NULL)
    {
    ... when != if (E == NULL) S1 else S2
    when != E = E1
    * E->f
    ... when any
    return ...;
    }
    else S3
    //


    Signed-off-by: Julia Lawall

    ---

    diff -u -p a/drivers/net/au1000_eth.c b/drivers/net/au1000_eth.c
    --- a/drivers/net/au1000_eth.c 2008-04-27 11:41:11.000000000 +0200
    +++ b/drivers/net/au1000_eth.c 2008-05-12 09:32:54.000000000 +0200
    @@ -1242,7 +1242,7 @@ static irqreturn_t au1000_interrupt(int
    struct net_device *dev = (struct net_device *) dev_id;

    if (dev == NULL) {
    - printk(KERN_ERR "%s: isr: null dev ptr\n", dev->name);
    + printk(KERN_ERR "isr: null dev ptr\n");
    return IRQ_RETVAL(1);
    }

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [PATCH 3/6] drivers/net: remove null pointer dereference

    Julia Lawall :
    [...]
    > diff -u -p a/drivers/net/au1000_eth.c b/drivers/net/au1000_eth.c
    > --- a/drivers/net/au1000_eth.c 2008-04-27 11:41:11.000000000 +0200
    > +++ b/drivers/net/au1000_eth.c 2008-05-12 09:32:54.000000000 +0200
    > @@ -1242,7 +1242,7 @@ static irqreturn_t au1000_interrupt(int
    > struct net_device *dev = (struct net_device *) dev_id;
    >
    > if (dev == NULL) {
    > - printk(KERN_ERR "%s: isr: null dev ptr\n", dev->name);
    > + printk(KERN_ERR "isr: null dev ptr\n");
    > return IRQ_RETVAL(1);
    > }


    The lifespan of 'dev' covers the request_irq..free_irq interval in this
    driver. The whole 'dev == NULL' block can be removed.

    --
    Ueimor
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH 3/6] drivers/net: remove null pointer dereference

    On Mon, 12 May 2008, Francois Romieu wrote:

    > Julia Lawall :
    > [...]
    > > diff -u -p a/drivers/net/au1000_eth.c b/drivers/net/au1000_eth.c
    > > --- a/drivers/net/au1000_eth.c 2008-04-27 11:41:11.000000000 +0200
    > > +++ b/drivers/net/au1000_eth.c 2008-05-12 09:32:54.000000000 +0200
    > > @@ -1242,7 +1242,7 @@ static irqreturn_t au1000_interrupt(int
    > > struct net_device *dev = (struct net_device *) dev_id;
    > >
    > > if (dev == NULL) {
    > > - printk(KERN_ERR "%s: isr: null dev ptr\n", dev->name);
    > > + printk(KERN_ERR "isr: null dev ptr\n");
    > > return IRQ_RETVAL(1);
    > > }

    >
    > The lifespan of 'dev' covers the request_irq..free_irq interval in this
    > driver. The whole 'dev == NULL' block can be removed.


    Will you do that?

    julia
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH 3/6] drivers/net: remove null pointer dereference

    Julia Lawall wrote:
    > On Mon, 12 May 2008, Francois Romieu wrote:
    >
    >> Julia Lawall :
    >> [...]
    >>> diff -u -p a/drivers/net/au1000_eth.c b/drivers/net/au1000_eth.c
    >>> --- a/drivers/net/au1000_eth.c 2008-04-27 11:41:11.000000000 +0200
    >>> +++ b/drivers/net/au1000_eth.c 2008-05-12 09:32:54.000000000 +0200
    >>> @@ -1242,7 +1242,7 @@ static irqreturn_t au1000_interrupt(int
    >>> struct net_device *dev = (struct net_device *) dev_id;
    >>>
    >>> if (dev == NULL) {
    >>> - printk(KERN_ERR "%s: isr: null dev ptr\n", dev->name);
    >>> + printk(KERN_ERR "isr: null dev ptr\n");
    >>> return IRQ_RETVAL(1);
    >>> }

    >> The lifespan of 'dev' covers the request_irq..free_irq interval in this
    >> driver. The whole 'dev == NULL' block can be removed.

    >
    > Will you do that?


    It's normal within the Linux community to give feedback on patches, and
    sometimes the authors need to revise their patches if helpful feedback
    arises.

    Jeff



    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. [PATCH] au1000_eth: remove useless check

    The lifespan of the device covers the request_irq .. free_irq interval.

    The cast of a void * pointer is not needed either.

    Signed-off-by: Francois Romieu

    diff --git a/drivers/net/au1000_eth.c b/drivers/net/au1000_eth.c
    index 3634b5f..7023d77 100644
    --- a/drivers/net/au1000_eth.c
    +++ b/drivers/net/au1000_eth.c
    @@ -1239,12 +1239,7 @@ static int au1000_rx(struct net_device *dev)
    */
    static irqreturn_t au1000_interrupt(int irq, void *dev_id)
    {
    - struct net_device *dev = (struct net_device *) dev_id;
    -
    - if (dev == NULL) {
    - printk(KERN_ERR "%s: isr: null dev ptr\n", dev->name);
    - return IRQ_RETVAL(1);
    - }
    + struct net_device *dev = dev_id;

    /* Handle RX interrupts first to minimize chance of overrun */

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [PATCH 3/6] drivers/net: remove null pointer dereference

    On Mon, 12 May 2008, Jeff Garzik wrote:

    > Julia Lawall wrote:
    > > On Mon, 12 May 2008, Francois Romieu wrote:
    > >
    > > > Julia Lawall :
    > > > [...]
    > > > > diff -u -p a/drivers/net/au1000_eth.c b/drivers/net/au1000_eth.c
    > > > > --- a/drivers/net/au1000_eth.c 2008-04-27 11:41:11.000000000 +0200
    > > > > +++ b/drivers/net/au1000_eth.c 2008-05-12 09:32:54.000000000 +0200
    > >>> @@ -1242,7 +1242,7 @@ static irqreturn_t au1000_interrupt(int
    > > > > struct net_device *dev = (struct net_device *) dev_id;
    > > > >
    > > > > if (dev == NULL) {
    > > > > - printk(KERN_ERR "%s: isr: null dev ptr\n", dev->name);
    > > > > + printk(KERN_ERR "isr: null dev ptr\n");
    > > > > return IRQ_RETVAL(1);
    > > > > }
    > > > The lifespan of 'dev' covers the request_irq..free_irq interval in this
    > > > driver. The whole 'dev == NULL' block can be removed.

    > >
    > > Will you do that?

    >
    > It's normal within the Linux community to give feedback on patches, and
    > sometimes the authors need to revise their patches if helpful feedback arises.


    Sorry. In principle, I would be happy to do it, but I felt slightly
    uneasy about doing something I didn't completely understand.

    julia
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: [PATCH] au1000_eth: remove useless check

    Francois Romieu wrote:
    > The lifespan of the device covers the request_irq .. free_irq interval.
    >
    > The cast of a void * pointer is not needed either.
    >
    > Signed-off-by: Francois Romieu
    >
    > diff --git a/drivers/net/au1000_eth.c b/drivers/net/au1000_eth.c
    > index 3634b5f..7023d77 100644
    > --- a/drivers/net/au1000_eth.c
    > +++ b/drivers/net/au1000_eth.c
    > @@ -1239,12 +1239,7 @@ static int au1000_rx(struct net_device *dev)
    > */
    > static irqreturn_t au1000_interrupt(int irq, void *dev_id)
    > {
    > - struct net_device *dev = (struct net_device *) dev_id;
    > -
    > - if (dev == NULL) {
    > - printk(KERN_ERR "%s: isr: null dev ptr\n", dev->name);
    > - return IRQ_RETVAL(1);
    > - }
    > + struct net_device *dev = dev_id;
    >


    applied


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread