Re: [static analysis] memory corruption in tty/synclink.c on 64bit

[Alan Cox]

On Mon, 17 Sep 2012 14:18:19 +0300
Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:

> I don't think anyone really cares about this but just for laughs here
> is the warning:
> 
> drivers/tty/synclink.c:5418 usc_process_rxoverrun_sync() warn: potential memory corrupting cast 8 vs 2 bytes
> drivers/tty/synclink.c:6414 mgsl_reset_tx_dma_buffers() warn: potential memory corrupting cast 8 vs 2 bytes
> drivers/tty/synclink.c:6455 mgsl_reset_rx_dma_buffers() warn: potential memory corrupting cast 8 vs 2 bytes
> drivers/tty/synclink.c:6500 mgsl_free_rx_frame_buffers() warn: potential memory corrupting cast 8 vs 2 bytes
> 
>   6489                  /* reset current buffer for reuse */
>   6490  //              pBufEntry->status = 0;
>   6491  //              pBufEntry->count = DMABUFFERSIZE;
>   6492                  *((unsigned long *)&(pBufEntry->count)) = DMABUFFERSIZE;
> 
> It looks like the commented out code is the correct code and the code
> on line 6492 is the "faster" version?  My static checker complains that
> it would corrupt the next struct elements on 64 bit.  Also it only works
> on little endian systems.
> 
> typedef struct _DMABUFFERENTRY
> {
>         u32 phys_addr;  /* 32-bit flat physical address of data buffer */
>         volatile u16 count;     /* buffer size/data count */
>         volatile u16 status;    /* Control/status field */
>         volatile u16 rcc;       /* character count field */
>         u16 reserved;   /* padding required by 16C32 */
> 
> This was there in the original code when it was merged in 2.2.5pre1.

Should probably be changed to *((u32 *) at least. As it happens as far as
I can see its harmless however.

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