[PATCH v4.19-v5.4] vt: fix memory overlapping when deleting chars in the buffer

[PATCH v4.19-v5.4] vt: fix memory overlapping when deleting chars in the buffer

[Kuntal Nayak]

From: Yangxi Xiang <xyangxi5@gmail.com>

[ upstream commit 39cdb68c64d8 ]

A memory overlapping copy occurs when deleting a long line. This memory
overlapping copy can cause data corruption when scr_memcpyw is optimized
to memcpy because memcpy does not ensure its behavior if the destination
buffer overlaps with the source buffer. The line buffer is not always
broken, because the memcpy utilizes the hardware acceleration, whose
result is not deterministic.

Fix this problem by using replacing the scr_memcpyw with scr_memmovew.

Fixes: 81732c3b2fed ("tty vt: Fix line garbage in virtual console on command line edition")
Cc: stable <stable@kernel.org>
Signed-off-by: Yangxi Xiang <xyangxi5@gmail.com>
Link: https://lore.kernel.org/r/20220628093322.5688-1-xyangxi5@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
[ KN: vc_state is not a separate structure in LTS v4.19, v5.4. Adjusted the patch
  accordingly by using vc_x instead of state.x for backport. ]
Signed-off-by: Kuntal Nayak <kuntal.nayak@broadcom.com>
---
 drivers/tty/vt/vt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index c9083d853..a351e264d 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -855,7 +855,7 @@ static void delete_char(struct vc_data *vc, unsigned int nr)
 	unsigned short *p = (unsigned short *) vc->vc_pos;
 
 	vc_uniscr_delete(vc, nr);
-	scr_memcpyw(p, p + nr, (vc->vc_cols - vc->vc_x - nr) * 2);
+	scr_memmovew(p, p + nr, (vc->vc_cols - vc->vc_x - nr) * 2);
 	scr_memsetw(p + vc->vc_cols - vc->vc_x - nr, vc->vc_video_erase_char,
 			nr * 2);
 	vc->vc_need_wrap = 0;
-- 
2.39.0

Re: [PATCH v4.19-v5.4] vt: fix memory overlapping when deleting chars in the buffer

[Greg KH]

On Fri, Mar 29, 2024 at 12:12:08PM -0700, Kuntal Nayak wrote:
> From: Yangxi Xiang <xyangxi5@gmail.com>
> 
> [ upstream commit 39cdb68c64d8 ]

Please use the full git id in the future, it makes it easier and our
tools want it that way so that we can tell the difference between a
"Fixes:" tag and an actual backport.  I'll go queue this up now, thanks!

greg k-h

Re: [PATCH v4.19-v5.4] vt: fix memory overlapping when deleting chars in the buffer

[Ajay Kaher]

On Sat, Mar 30, 2024 at 2:37 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Fri, Mar 29, 2024 at 12:12:08PM -0700, Kuntal Nayak wrote:
> > From: Yangxi Xiang <xyangxi5@gmail.com>
> >
> > [ upstream commit 39cdb68c64d8 ]
>
> Please use the full git id in the future, it makes it easier and our
> tools want it that way so that we can tell the difference between a
> "Fixes:" tag and an actual backport.  I'll go queue this up now, thanks!
>

Thanks Greg for accepting the patch.

./scripts/checkpatch.pl confuse developers, as it gives error/warning
if we use full git id.

Example:
ERROR: Please use git commit description style 'commit <12+ chars of
sha1> ("<title line>")' - ie: 'commit e26d3009efda ("netfilter:
nf_tables: disallow timeout for anonymous sets")'
#6:
commit e26d3009efda338f19016df4175f354a9bd0a4ab upstream.

Or shall we have something ./scripts/checkpatchstable.pl or
./scripts/checkpatch.pl stable (pass argument)?

-Ajay

Re: [PATCH v4.19-v5.4] vt: fix memory overlapping when deleting chars in the buffer

[Greg KH]

On Mon, Apr 01, 2024 at 02:51:55PM +0530, Ajay Kaher wrote:
> On Sat, Mar 30, 2024 at 2:37 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Mar 29, 2024 at 12:12:08PM -0700, Kuntal Nayak wrote:
> > > From: Yangxi Xiang <xyangxi5@gmail.com>
> > >
> > > [ upstream commit 39cdb68c64d8 ]
> >
> > Please use the full git id in the future, it makes it easier and our
> > tools want it that way so that we can tell the difference between a
> > "Fixes:" tag and an actual backport.  I'll go queue this up now, thanks!
> >
> 
> Thanks Greg for accepting the patch.
> 
> ./scripts/checkpatch.pl confuse developers, as it gives error/warning
> if we use full git id.

No need to run checkpatch on backports, as the changelog text should not
be an issue.

> Example:
> ERROR: Please use git commit description style 'commit <12+ chars of
> sha1> ("<title line>")' - ie: 'commit e26d3009efda ("netfilter:
> nf_tables: disallow timeout for anonymous sets")'
> #6:
> commit e26d3009efda338f19016df4175f354a9bd0a4ab upstream.
> 
> Or shall we have something ./scripts/checkpatchstable.pl or
> ./scripts/checkpatch.pl stable (pass argument)?

Nah, it's never come up before now, I don't think people actually run
it, and I know I've never done so or even noticed, so all is fine.

thanks,

greg k-h