| Message ID | 20190905184935.30694-1-paulo@paulo.ac |
|---|---|
| State | New |
| Headers | show |
| Series | mount.cifs: Fix double-free issue when mounting with setuid root | expand |
" Paulo Alcantara (SUSE) " <paulo@paulo.ac> writes: > It can be easily reproduced with the following: > > # chmod +s `which mount.cifs` > # echo "//localhost/share /mnt cifs \ > users,username=foo,password=XXXX" >> /etc/fstab > # su - foo > $ mount /mnt > free(): double free detected in tcache 2 > Child process terminated abnormally. > > The problem was that check_fstab() already freed orgoptions pointer > and then we freed it again in main() function. > > Fixes: bf7f48f4c7dc ("mount.cifs.c: fix memory leaks in main func") > Signed-off-by: Paulo Alcantara (SUSE) <paulo@paulo.ac> Reviewed-by: Aurelien Aptel <aaptel@suse.com> I've compiled next branch with ASAN and can confirm the double-free and the fix works Compiling with ASAN ------------------- $ CFLAGS=-fsanitize=address \ LDFLAGS=-fsanitize=address \ ac_cv_func_malloc_0_nonnull=yes \ ./configure $ make clean && make -j4 Next branch ----------- $ mount /mnt; echo $? ================================================================= ==29883==ERROR: AddressSanitizer: attempting double-free on 0x607000000020 in thread T0: #0 0x7f69d480a1b8 in __interceptor_free (/usr/lib64/libasan.so.4+0xdc1b8) #1 0x559381795f33 in main (/sbin/mount.cifs+0xef33) #2 0x7f69d4394f89 in __libc_start_main (/lib64/libc.so.6+0x20f89) #3 0x55938178e079 in _start (/sbin/mount.cifs+0x7079) 0x607000000020 is located 0 bytes inside of 68-byte region [0x607000000020,0x607000000064) freed by thread T0 here: #0 0x7f69d480a1b8 in __interceptor_free (/usr/lib64/libasan.so.4+0xdc1b8) #1 0x55938178e372 in check_fstab (/sbin/mount.cifs+0x7372) #2 0x559381794661 in assemble_mountinfo (/sbin/mount.cifs+0xd661) #3 0x559381795eef in main (/sbin/mount.cifs+0xeeef) #4 0x7f69d4394f89 in __libc_start_main (/lib64/libc.so.6+0x20f89) previously allocated by thread T0 here: #0 0x7f69d480a510 in malloc (/usr/lib64/libasan.so.4+0xdc510) #1 0x7f69d43fc2a9 in __GI___strndup (/lib64/libc.so.6+0x882a9) SUMMARY: AddressSanitizer: double-free (/usr/lib64/libasan.so.4+0xdc1b8) in __interceptor_free ==29883==ABORTING 1 With fix -------- $ mount /mnt; echo $? 0 Cheers,
пн, 9 сент. 2019 г. в 05:46, Aurélien Aptel <aaptel@suse.com>: > > " Paulo Alcantara (SUSE) " <paulo@paulo.ac> writes: > > It can be easily reproduced with the following: > > > > # chmod +s `which mount.cifs` > > # echo "//localhost/share /mnt cifs \ > > users,username=foo,password=XXXX" >> /etc/fstab > > # su - foo > > $ mount /mnt > > free(): double free detected in tcache 2 > > Child process terminated abnormally. > > > > The problem was that check_fstab() already freed orgoptions pointer > > and then we freed it again in main() function. > > > > Fixes: bf7f48f4c7dc ("mount.cifs.c: fix memory leaks in main func") > > Signed-off-by: Paulo Alcantara (SUSE) <paulo@paulo.ac> > > Reviewed-by: Aurelien Aptel <aaptel@suse.com> > > I've compiled next branch with ASAN and can confirm the double-free and > the fix works > > Compiling with ASAN > ------------------- > > $ CFLAGS=-fsanitize=address \ > LDFLAGS=-fsanitize=address \ > ac_cv_func_malloc_0_nonnull=yes \ > ./configure > $ make clean && make -j4 > > Next branch > ----------- > > $ mount /mnt; echo $? > ================================================================= > ==29883==ERROR: AddressSanitizer: attempting double-free on 0x607000000020 in thread T0: > #0 0x7f69d480a1b8 in __interceptor_free (/usr/lib64/libasan.so.4+0xdc1b8) > #1 0x559381795f33 in main (/sbin/mount.cifs+0xef33) > #2 0x7f69d4394f89 in __libc_start_main (/lib64/libc.so.6+0x20f89) > #3 0x55938178e079 in _start (/sbin/mount.cifs+0x7079) > > 0x607000000020 is located 0 bytes inside of 68-byte region [0x607000000020,0x607000000064) > freed by thread T0 here: > #0 0x7f69d480a1b8 in __interceptor_free (/usr/lib64/libasan.so.4+0xdc1b8) > #1 0x55938178e372 in check_fstab (/sbin/mount.cifs+0x7372) > #2 0x559381794661 in assemble_mountinfo (/sbin/mount.cifs+0xd661) > #3 0x559381795eef in main (/sbin/mount.cifs+0xeeef) > #4 0x7f69d4394f89 in __libc_start_main (/lib64/libc.so.6+0x20f89) > > previously allocated by thread T0 here: > #0 0x7f69d480a510 in malloc (/usr/lib64/libasan.so.4+0xdc510) > #1 0x7f69d43fc2a9 in __GI___strndup (/lib64/libc.so.6+0x882a9) > > SUMMARY: AddressSanitizer: double-free (/usr/lib64/libasan.so.4+0xdc1b8) in __interceptor_free > ==29883==ABORTING > 1 > > With fix > -------- > > $ mount /mnt; echo $? > 0 > > Cheers, > -- > Aurélien Aptel / SUSE Labs Samba Team > GPG: 1839 CB5F 9F5B FB9B AA97 8C99 03C8 A49B 521B D5D3 > SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE > GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München) Merged, thanks. -- Best regards, Pavel Shilovsky
diff --git a/mount.cifs.c b/mount.cifs.c index 7748d54aa814..2116fc803311 100644 --- a/mount.cifs.c +++ b/mount.cifs.c @@ -247,7 +247,6 @@ check_fstab(const char *progname, const char *mountpoint, const char *devname, * set of options. We don't want to trust what the user * gave us, so just take whatever is in /etc/fstab. */ - free(*options); *options = strdup(mnt->mnt_opts); return 0; } @@ -1762,6 +1761,7 @@ assemble_mountinfo(struct parsed_mount_info *parsed_info, const char *orig_dev, char *orgoptions) { int rc; + char *newopts = NULL; rc = drop_capabilities(0); if (rc) @@ -1773,10 +1773,11 @@ assemble_mountinfo(struct parsed_mount_info *parsed_info, if (getuid()) { rc = check_fstab(thisprogram, mountpoint, orig_dev, - &orgoptions); + &newopts); if (rc) goto assemble_exit; + orgoptions = newopts; /* enable any default user mount flags */ parsed_info->flags |= CIFS_SETUID_FLAGS; } @@ -1880,6 +1881,7 @@ assemble_mountinfo(struct parsed_mount_info *parsed_info, } assemble_exit: + free(newopts); return rc; }
It can be easily reproduced with the following: # chmod +s `which mount.cifs` # echo "//localhost/share /mnt cifs \ users,username=foo,password=XXXX" >> /etc/fstab # su - foo $ mount /mnt free(): double free detected in tcache 2 Child process terminated abnormally. The problem was that check_fstab() already freed orgoptions pointer and then we freed it again in main() function. Fixes: bf7f48f4c7dc ("mount.cifs.c: fix memory leaks in main func") Signed-off-by: Paulo Alcantara (SUSE) <paulo@paulo.ac> --- mount.cifs.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)