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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20120917141636.67e4b924@pyramind.ukuu.org.uk>
Date:	Mon, 17 Sep 2012 14:16:36 +0100
From:	Alan Cox <alan@...rguk.ukuu.org.uk>
To:	Dan Carpenter <dan.carpenter@...cle.com>
Cc:	linux-kernel@...r.kernel.org, Jiri Slaby <jslaby@...e.cz>
Subject: Re: [static analysis] memory corruption in tty/synclink.c on 64bit

On Mon, 17 Sep 2012 14:18:19 +0300
Dan Carpenter <dan.carpenter@...cle.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.

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