Hi Mikael,
On Sat, Jun 17, 2006 at 10:47:53PM +0200, Mikael Pettersson wrote:
> On Fri, 16 Jun 2006 15:14:19 -0300, Marcelo Tosatti wrote:
> >Summary of changes from v2.4.33-pre3 to v2.4.33-rc1
> ...
> >Willy TARREAU:
> ...
> > ide: trying to enable DMA may cause an oops
>
> This patch to ide-dma.c defines a function 'int __ide_dma_no_op(void)'
> and stores its address in function pointer fields with type
> 'int (*)(ide_drive_t*)'. Thus callers will call __ide_dma_no_op() with
> more parameters than it expects.
>
> This is invalid C and it will break horribly in some valid calling
> conventions (in particular, when parameters are passed on the stack
> and the callee not the caller pops them).
I 100% agree with your analysis and you fix, but just out of curiosity,
when could we encounter such circumstances ? On specific archs or when
functions are declared in a certain manner ? I've always learned that
in C, it's always the caller which pops, reason why var args are possible
and the args type checking is very loose.
> Furtunately the fix is simple: just define __ide_dma_no_op() with
> the correct prototype (taking an unused ide_drive_t* parameter),
> and drop the now redundant casts from the assignments. Also make
> __ide_dma_no_op() 'static' as it is local to ide-dma.c.
Thank you very much, I've queued it in -upstream. Marcelo, I've updated
the URL, please use http://git.1wt.eu/linux-2.4-upstream.git as of now.
Cheers,
Willy
> Signed-off-by: Mikael Pettersson <[email protected]>
>
> diff -rupN linux-2.4.33-rc1/drivers/ide/ide-dma.c linux-2.4.33-rc1.ide-dma-no-op-fix/drivers/ide/ide-dma.c
> --- linux-2.4.33-rc1/drivers/ide/ide-dma.c 2006-06-17 17:58:36.000000000 +0200
> +++ linux-2.4.33-rc1.ide-dma-no-op-fix/drivers/ide/ide-dma.c 2006-06-17 18:12:31.000000000 +0200
> @@ -572,7 +572,7 @@ static int dma_timer_expiry (ide_drive_t
> * This empty function prevents non-DMA controllers from causing an oops.
> */
>
> -int __ide_dma_no_op (void)
> +static int __ide_dma_no_op (ide_drive_t *ignored)
> {
> return 0;
> }
> @@ -1235,11 +1235,11 @@ EXPORT_SYMBOL_GPL(ide_setup_dma);
> void ide_setup_no_dma (ide_hwif_t *hwif)
> {
> if (!hwif->ide_dma_off_quietly)
> - hwif->ide_dma_off_quietly = (int (*)(ide_drive_t *))&__ide_dma_no_op;
> + hwif->ide_dma_off_quietly = &__ide_dma_no_op;
> if (!hwif->ide_dma_host_off)
> - hwif->ide_dma_host_off = (int (*)(ide_drive_t *))&__ide_dma_no_op;
> + hwif->ide_dma_host_off = &__ide_dma_no_op;
> if (!hwif->ide_dma_host_on)
> - hwif->ide_dma_host_on = (int (*)(ide_drive_t *))&__ide_dma_no_op;
> + hwif->ide_dma_host_on = &__ide_dma_no_op;
> }
>
> EXPORT_SYMBOL_GPL(ide_setup_no_dma);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
