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

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

[Dan Carpenter]

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.

regards,
dan carpenter

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@oracle.com> 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.