linux-kernel - Re: [PATCH] vgacon: fix out of bounds write to the scrollback buffer

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <c1f267aa-dfb3-91fa-3111-30c1676f1a91@kernel.org>
Date:   Wed, 29 Jul 2020 10:11:57 +0200
From:   Jiri Slaby <jirislaby@...nel.org>
To:     张云海 <zhangyunhai@...ocus.com>,
        b.zolnierkie@...sung.com
Cc:     linux-kernel@...r.kernel.org,
        Yang Yingliang <yangyingliang@...wei.com>,
        Kyungtae Kim <kt0755@...il.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Greg KH <greg@...ah.com>, Solar Designer <solar@...nwall.com>,
        "Srivatsa S. Bhat" <srivatsa@...il.mit.edu>,
        Anthony Liguori <aliguori@...zon.com>,
        Security Officers <security@...nel.org>,
        linux-distros@...openwall.org, dri-devel@...ts.freedesktop.org,
        linux-fbdev@...r.kernel.org
Subject: Re: [PATCH] vgacon: fix out of bounds write to the scrollback buffer

Hi,

On 29. 07. 20, 9:53, 张云海 wrote:
> This patch dosen't fix the issue, the check should be in the loop.
> 
> The change of the VT sze is before vgacon_scrollback_update, not in the
> meantime.
> 
> Let's consider the following situation:
> 	suppose:
> 		vgacon_scrollback_cur->size is 65440
> 		vgacon_scrollback_cur->tail is 64960
> 		c->vc_size_row is 160
> 		count is 5
> 	
> 	Reset c->vc_size_row to 200 by VT_RESIZE, then call
> vgacon_scrollback_update.
> 	
> 	This will pass the check, since (vgacon_scrollback_cur->tail +
> c->vc_size_row)
> 	is 65160 which is less then vgacon_scrollback_cur->size(65440).
> 
> 	However, in the 3rd iteration of the loop, vgacon_scrollback_cur->tail
> is update
> 	to 65360, the memcpy will overflow.

But the loop checks for the overflow:
  if (vgacon_scrollback_cur->tail >= vgacon_scrollback_cur->size)
        vgacon_scrollback_cur->tail = 0;

So the first 2 iterations would write to the end of the buffer and this
3rd one should have zeroed ->tail.

thanks,
-- 
js
suse labs