Discussion:
iwi(4) patch
(too old to reply)
Jean-Yves Migeon
2009-03-08 00:22:19 UTC
Permalink
Hi,

With a Xen debug kernel (PAE off), iwi(4) is not able to attach during
autoconf ("XENMEM_decrease_reservation failed!" issue) on my laptop.
This should not be fatal to the boot process, but in fact, it is; errors
are (from my PoV) incorrectly handled in if_iwi.c.

I have local fixes to it [1]. However, as it is not a part of the kernel
I am familiar with, I would like more experienced people to review my patch:

- some bus_dma fix, with checks against invalid dmap.

- move the (RX and TX) rings' allocation code later in iwi_attach() to
avoid NULL pointer dereference if allocation fails (iwi_detach()
manipulates structure variables that are not set in case of a failed
allocation)

- avoid double free in case of failure during attach. If an allocation
fails, do not free it directly, this is handled by iwi_detach() routine.

- only set ring->count for RX/TX rings when ring malloc() is successful,
or else the for loop during detach will fail with a NULL dereference.

Tested with current GENERIC, XEN3_DOM0 and XEN3PAE_DOM0. Did not notice
any regression so far.

[1] http://www.netbsd.org/~jym/if_iwi.c.diff

Thanks!
--
Jean-Yves Migeon
***@free.fr


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Christos Zoulas
2009-03-08 01:35:29 UTC
Permalink
Post by Jean-Yves Migeon
Hi,
With a Xen debug kernel (PAE off), iwi(4) is not able to attach during
autoconf ("XENMEM_decrease_reservation failed!" issue) on my laptop.
This should not be fatal to the boot process, but in fact, it is; errors
are (from my PoV) incorrectly handled in if_iwi.c.
I have local fixes to it [1]. However, as it is not a part of the kernel
- some bus_dma fix, with checks against invalid dmap.
- move the (RX and TX) rings' allocation code later in iwi_attach() to
avoid NULL pointer dereference if allocation fails (iwi_detach()
manipulates structure variables that are not set in case of a failed
allocation)
- avoid double free in case of failure during attach. If an allocation
fails, do not free it directly, this is handled by iwi_detach() routine.
- only set ring->count for RX/TX rings when ring malloc() is successful,
or else the for loop during detach will fail with a NULL dereference.
Tested with current GENERIC, XEN3_DOM0 and XEN3PAE_DOM0. Did not notice
any regression so far.
[1] http://www.netbsd.org/~jym/if_iwi.c.diff
Looks ok to me.

christos


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
David Young
2009-03-08 03:37:11 UTC
Permalink
Post by Jean-Yves Migeon
Hi,
With a Xen debug kernel (PAE off), iwi(4) is not able to attach during
autoconf ("XENMEM_decrease_reservation failed!" issue) on my laptop.
This should not be fatal to the boot process, but in fact, it is; errors
are (from my PoV) incorrectly handled in if_iwi.c.
I have local fixes to it [1]. However, as it is not a part of the kernel
- some bus_dma fix, with checks against invalid dmap.
- move the (RX and TX) rings' allocation code later in iwi_attach() to
avoid NULL pointer dereference if allocation fails (iwi_detach()
manipulates structure variables that are not set in case of a failed
allocation)
- avoid double free in case of failure during attach. If an allocation
fails, do not free it directly, this is handled by iwi_detach() routine.
Judging by your patch alone, if iwi_attach() fails midway through,
it will leak resources, will it not?

Dave
--
David Young OJC Technologies
***@ojctech.com Urbana, IL * (217) 278-3933

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de
Loading...