Re: 2.4.23pre6aa2 - some problems [with patches]

[Thomas Winischhofer <thomas@winischhofer.net>]

Andrea Arcangeli wrote:
>>depmod: *** Unresolved symbols in
>>/lib/modules/2.4.23-pre6-aa2/kernel/drivers/vi
>>deo/sis/sisfb.o
>>depmod:         __floatsidf
>>depmod:         __divdf3
>>depmod:         __fixunsdfsi
>>depmod:         __muldf3
>>depmod:         __adddf3
> 
> 
> those drivers are buggy, and had always been buggy, it's just that you
> couldn't notice it before. I added an option in my new tree to catch
> those longstanding bugs. They *must* not compile. Driver
> authors can audit their 2.4 drivers by grabbing my tree or by applying
> this patch alone:
> 
> 	http://www.us.kernel.org/pub/linux/kernel/people/andrea/kernels/v2.4/2.4.23pre6aa2/9999900_soft-float-1
> 
> see drivers/video/sis/init.c:
> 
>       divider = (sr2b & 0x80) ? 2.0 : 1.0;
>       postscalar = (sr2c & 0x80) ?
>               ( (((sr2c >> 5) & 0x03) == 0x02) ? 6.0 : 8.0) : (((sr2c >> 5) & 0x03) + 1.0);
>       num = (sr2b & 0x7f) + 1.0;
>       denum = (sr2c & 0x1f) + 1.0;
> 
> there would be a slight chance to find false positives too, but those
> aren't false positives, there's no fpu save at all there.
> 
> 
> Ironically all the float numbers seems to end with .0, I fixed it a bit
> but I've no idea if I catched all them, so Eyal, you can try to apply
> this patch and follow the code to see if you can spot more of these
> longstanding bugs (those could corrupt the fpu state and segfault
> userspace at the very least):
> 
> --- xx/drivers/video/sis/init.c.~1~	2003-10-02 00:09:44.000000000 +0200
> +++ xx/drivers/video/sis/init.c	2003-10-05 12:36:03.000000000 +0200
> @@ -3940,11 +3940,11 @@ SiSBuildBuiltInModeList(ScrnInfoPtr pScr
>        sr2b = pSiS->SiS_Pr->SiS_VCLKData[vclkindex].SR2B;
>        sr2c = pSiS->SiS_Pr->SiS_VCLKData[vclkindex].SR2C;
>  
> -      divider = (sr2b & 0x80) ? 2.0 : 1.0;
> +      divider = (sr2b & 0x80) ? 2 : 1;
>        postscalar = (sr2c & 0x80) ?
> -              ( (((sr2c >> 5) & 0x03) == 0x02) ? 6.0 : 8.0) : (((sr2c >> 5) & 0x03) + 1.0);
> -      num = (sr2b & 0x7f) + 1.0;
> -      denum = (sr2c & 0x1f) + 1.0;
> +              ( (((sr2c >> 5) & 0x03) == 0x02) ? 6 : 8) : (((sr2c >> 5) & 0x03) + 1);
> +      num = (sr2b & 0x7f) + 1;
> +      denum = (sr2c & 0x1f) + 1;
>  
>  #ifdef TWDEBUG
>        xf86DrvMsg(0, X_INFO, "------------\n");
> 

That part of init.c doesn't get compiled in the linux kernel, it's for 
XFree86. I think the xf86DrvMsg statement should make that clear...

Thomas

-- 
Thomas Winischhofer
Vienna/Austria
thomas AT winischhofer DOT net          http://www.winischhofer.net/
twini AT xfree86 DOT org