info-mtools
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Info-mtools] Small leaks in mtools, patches included


From: Pavel Cahyna
Subject: [Info-mtools] Small leaks in mtools, patches included
Date: Wed, 4 Sep 2024 18:28:05 +0200

Hello all,

Coverity found several leaks in mtools. One is missing iconv_close()
call after iconv_open() in charsetConv.c, function try(). It can be
observed in Valgrind:

valgrind --leak-check=full --track-origins=yes ./mdir -i vfat.img ::/stuff

Before the fix:

==578826== 272 (64 direct, 208 indirect) bytes in 1 blocks are definitely lost 
in loss record 9 of 11
==578826==    at 0x484280F: malloc (vg_replace_malloc.c:442)
==578826==    by 0x48A5500: __gconv_open (gconv_open.c:77)
==578826==    by 0x48A5069: iconv_open (iconv_open.c:39)
==578826==    by 0x404650: try (charsetConv.c:64)
==578826==    by 0x404650: getWcharCp (charsetConv.c:91)
==578826==    by 0x404733: cp_open (charsetConv.c:113)
==578826==    by 0x40C86F: fs_init (init.c:591)
==578826==    by 0x41C5FE: open_root_dir (streamcache.c:65)
==578826==    by 0x40D780: common_dos_loop (mainloop.c:437)
==578826==    by 0x40DE7C: dos_loop (mainloop.c:464)
==578826==    by 0x40DE7C: main_loop (mainloop.c:537)
==578826==    by 0x411B9B: mdir (mdir.c:617)
==578826==    by 0x403A0A: main (mtools.c:170)
==578826== 
==578826== LEAK SUMMARY:
==578826==    definitely lost: 176 bytes in 3 blocks
==578826==    indirectly lost: 208 bytes in 1 blocks
==578826==      possibly lost: 0 bytes in 0 blocks
==578826==    still reachable: 2,727 bytes in 8 blocks
==578826==         suppressed: 0 bytes in 0 blocks
==578826== Reachable blocks (those to which a pointer was found) are not shown.
==578826== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==578826== 
==578826== For lists of detected and suppressed errors, rerun with: -s
==578826== ERROR SUMMARY: 52 errors from 7 contexts (suppressed: 0 from 0)

After the fix:

==578861== LEAK SUMMARY:
==578861==    definitely lost: 112 bytes in 2 blocks
==578861==    indirectly lost: 0 bytes in 0 blocks
==578861==      possibly lost: 0 bytes in 0 blocks
==578861==    still reachable: 2,727 bytes in 8 blocks
==578861==         suppressed: 0 bytes in 0 blocks
==578861== Reachable blocks (those to which a pointer was found) are not shown.
==578861== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==578861==
==578861== For lists of detected and suppressed errors, rerun with: -s
==578861== ERROR SUMMARY: 51 errors from 6 contexts (suppressed: 0 from 0)

Fix is attached as iconv_close.patch.

The other is missing close() in mpartition.c, function mpartition(),
leading to a file descriptor leak. The bug can be observed using strace
(the reproducer below assumes Fedora, should be easy to change for other
distros by adjusting the package manager and package names), and assumes
an unused /dev/vdb disk that can be used for experiments:

yum -y install mtools strace syslinux-nonlinux
echo 'drive c: file="/dev/vdb" partition=1' >> /etc/mtools.conf
dd if=/usr/share/syslinux/mbr.bin of=mbr.bin count=1 bs=512 conv=sync
strace -e %file,%desc mpartition -I -B mbr.bin c:

...
openat(AT_FDCWD, "/dev/vdb", O_RDWR|O_CREAT, 0666) = 3
fstat(3, {st_mode=S_IFBLK|0660, st_rdev=makedev(0xfd, 0x10), ...}) = 0
ioctl(3, HDIO_GETGEO, {heads=16, sectors=63, cylinders=16383, start=0}) = 0
ioctl(3, BLKGETSIZE, [20971520])        = 0
ioctl(3, BLKSSZGET, [512])              = 0
read(3, "\353X\220mkfs.fat\0\2\20 \0\2\0\0\0\0\370\0\0?\0\377\0\0\0\0\0"..., 
512) = 512
openat(AT_FDCWD, "mbr.bin", O_RDONLY)   = 4
read(4, 
"3\300\372\216\330\216\320\274\0|\211\346\6W\216\300\373\374\277\0\6\271\0\1\363\245\352\37\6\0\0R"...,
 512) = 512
write(2, "Warning: no active (bootable) pa"..., 48Warning: no active (bootable) 
partition present
) = 48
lseek(3, 0, SEEK_SET)                   = 0
write(3, 
"3\300\372\216\330\216\320\274\0|\211\346\6W\216\300\373\374\277\0\6\271\0\1\363\245\352\37\6\0\0R"...,
 512) = 512
close(3)                                = 0
+++ exited with 0 +++

Note that "close(4)" is missing.

Patch is attached as close.patch .

Regards, Pavel Cahyna

Attachment: iconv_close.patch
Description: Text document

Attachment: close.patch
Description: Text document


reply via email to

[Prev in Thread] Current Thread [Next in Thread]