netdev - Re: [PATCH] korina: Read buffer overflow

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 8 Aug 2009 18:45:05 +0200
From:	Phil Sutter <n0-1@...ewrt.org>
To:	roel kluin <roel.kluin@...il.com>
Cc:	netdev@...r.kernel.org, Andrew Morton <akpm@...ux-foundation.org>,
	"David S. Miller" <davem@...emloft.net>, florian@...nwrt.org
Subject: Re: [PATCH] korina: Read buffer overflow

Hi,

On Sat, Aug 08, 2009 at 03:14:57PM +0200, roel kluin wrote:
> A few comments:
> 
> > | diff --git a/drivers/net/korina.c b/drivers/net/korina.c
> > | index a2701f5..d1c5276 100644
> > | --- a/drivers/net/korina.c
> > | +++ b/drivers/net/korina.c
> 
> > | @@ -772,6 +772,13 @@ static void korina_alloc_ring(struct net_device *dev)
> > |               lp->rd_ring[i].ca = CPHYSADDR(skb->data);
> > |               lp->rd_ring[i].link = CPHYSADDR(&lp->rd_ring[i+1]);
> > |       }
> > | +     if (!i) {
> > | +             printk(KERN_ERR DRV_NAME "%s: could not allocate a single"
> > | +                             " receive descriptor\n", dev->name)
> 
>             printk(KERN_ERR "%s: could not allocate a single receive
> descriptor\n",
>                             dev->name);
> 
> Don't interrupt strings, they are an exception in the 80 character
> limit (since they are easier to grep that way).

Ah, good to know.

> Also I think the DRV_NAME should be omitted, Doesn't
> `DRV_NAME "%s:...\n", dev->name)' result in "korinakorina:..."? It
> occurs at more places in the file. Also a semicolon was missing.

Sounds like a valid point, I will check this. This patch was just a
quick hack to illustrate what I had in mind, without even
compile-testing it.

> Otherwise looks fine to me.

Great you like it, and many thanks for reviewing it. I'm still not
completely sure the NIC will still work with less than 64 receive
descriptors, but since the documentation wasn't really enlightening I'll
add some run-time test to my TODO. Maybe one of the other readers has
some advice here, I'll prepare a complete patch in the meantime.

Greetings, Phil


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html