cifs: fix memory corruption setting EAs on 32 bit systems

Commit Message

Steve French Oct. 10, 2020, 10:54 p.m. UTC

Original patch was corrupted.   Fixed the whitespace/tab and
formatting issues and added cc:stable.

Merged into cifs-2.6.git for-next pending testing/review

Vladimir,
Would you verify that the updated patch matches what you expect?
Probably easier to send future patches as attachments or links to git
tree commit to avoid the usual email corruption of non-plain text
patches.

Comments

Vladimir Zapolskiy Oct. 11, 2020, 7:45 a.m. UTC | #1

Hi Steve,

On 10/11/20 1:54 AM, Steve French wrote:
> Original patch was corrupted.   Fixed the whitespace/tab and
> formatting issues and added cc:stable.
> 
> Merged into cifs-2.6.git for-next pending testing/review

thank you so much!

> Vladimir,
> Would you verify that the updated patch matches what you expect?

Certainly, I've compared my original patch with the commit on the
for-next branch, and the code change itself is completely equal to mine.

> Probably easier to send future patches as attachments or links to git
> tree commit to avoid the usual email corruption of non-plain text
> patches.
> 

Here the patch is a regular plain test file created by git-format-patch
utility and sent by git-send-email. Apparently any issues were caused on
the next stage of editing the commit message, that's what the diff says.

Thank you again for review, I have a few more changes in my queue, they
are not as critical as this one, thus I'll send them at a slow rate.

--
Best wishes,
Vladimir

Patch

From 5c119c376e10f4e943d143d42defb4e0e1bc64e3 Mon Sep 17 00:00:00 2001
From: Vladimir Zapolskiy <[e-mail]>
Date: Sat, 10 Oct 2020 17:44:18 -0500
Subject: [PATCH] cifs: fix memory corruption setting EAs on 32 bit systems

On setxattr() syscall path due to an apprent typo the size of a dynamically
allocated memory chunk for storing struct smb2_file_full_ea_info object is
computed incorrectly, to be more precise the first addend is the size of
a pointer instead of the wanted object size. Coincidentally it makes no
difference on 64-bit platforms, however on 32-bit targets the following
memcpy() writes 4 bytes of data outside of the dynamically allocated memory.

  BUG kmalloc-16 (Not tainted): Redzone overwritten
  -----------------------------------------------------------------------------

  Disabling lock debugging due to kernel taint
  INFO: 0x79e69a6f-0x9e5cdecf @offset=368. First byte 0x73 instead of 0xcc
  INFO: Slab 0xd36d2454 objects=85 used=51 fp=0xf7d0fc7a flags=0x35000201
  INFO: Object 0x6f171df3 @offset=352 fp=0x00000000

  Redzone 5d4ff02d: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc  ................
  Object 6f171df3: 00 00 00 00 00 05 06 00 73 6e 72 75 62 00 66 69  ........snrub.fi
  Redzone 79e69a6f: 73 68 32 0a                                      sh2.
  Padding 56254d82: 5a 5a 5a 5a 5a 5a 5a 5a                          ZZZZZZZZ
  CPU: 0 PID: 8196 Comm: attr Tainted: G    B             5.9.0-rc8+ #3
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1 04/01/2014
  Call Trace:
   dump_stack+0x54/0x6e
   print_trailer+0x12c/0x134
   check_bytes_and_report.cold+0x3e/0x69
   check_object+0x18c/0x250
   free_debug_processing+0xfe/0x230
   __slab_free+0x1c0/0x300
   kfree+0x1d3/0x220
   smb2_set_ea+0x27d/0x540
   cifs_xattr_set+0x57f/0x620
   __vfs_setxattr+0x4e/0x60
   __vfs_setxattr_noperm+0x4e/0x100
   __vfs_setxattr_locked+0xae/0xd0
   vfs_setxattr+0x4e/0xe0
   setxattr+0x12c/0x1a0
   path_setxattr+0xa4/0xc0
   __ia32_sys_lsetxattr+0x1d/0x20
   __do_fast_syscall_32+0x40/0x70
   do_fast_syscall_32+0x29/0x60
   do_SYSENTER_32+0x15/0x20
   entry_SYSENTER_32+0x9f/0xf2

Fixes: 5517554e4313 ("cifs: Add support for writing attributes on SMB2+")
Signed-off-by: Vladimir Zapolskiy <[e-mail]>
CC: Stable <[e-mail]> #v4.14+
Signed-off-by: Steve French <[e-mail]>
---
 fs/cifs/smb2ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 24f107f763f0..76d82a60a550 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -1216,7 +1216,7 @@  smb2_set_ea(const unsigned int xid, struct cifs_tcon *tcon,
 	rqst[1].rq_iov = si_iov;
 	rqst[1].rq_nvec = 1;
 
-	len = sizeof(ea) + ea_name_len + ea_value_len + 1;
+	len = sizeof(*ea) + ea_name_len + ea_value_len + 1;
 	ea = kzalloc(len, GFP_KERNEL);
 	if (ea == NULL) {
 		rc = -ENOMEM;
-- 
2.25.1