SANE Backend Memory Leak

[Ariel S]

Hello, I found a leak in HPLIP when scanning. My distribution is ArchLinux,
HPLIP version 3.13.11, my device is HP Deskjet F2276 All-in-One.

I found the problem when using SANE to do scanning using SANE's scanimage.

I've built SANE with debug information and disable library unloading
to pin dowh the source, this is what I get from valgrind:

    $ valgrind --leak-check=full --leak-resolution=high --track-origins=yes scanimage > /dev/null
    ==9049== Memcheck, a memory error detector
    ==9049== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
    ==9049== Using Valgrind-3.9.0 and LibVEX; rerun with -h for copyright info
    ==9049== Command: scanimage
    ==9049==
    ==9049== Conditional jump or move depends on uninitialised value(s)
    ==9049== at 0xA688A20: GetPair (model.c:73)
    ==9049== by 0xA688B8A: ReadConfig (model.c:123)
    ==9049== by 0xA6895A2: hpmud_get_model_attributes (model.c:522)
    ==9049== by 0xA6896C8: hpmud_query_model (model.c:563)
    ==9049== by 0xA691CB8: musb_probe_devices (musb.c:2100)
    ==9049== by 0xA68742C: hpmud_probe_devices (hpmud.c:598)
    ==9049== by 0xA23A1A4: DevDiscovery (in /usr/lib/sane/libsane-hpaio.so.1.0.0)
    ==9049== by 0xA23A7A8: sane_hpaio_get_devices (in /usr/lib/sane/libsane-hpaio.so.1.0.0)
    ==9049== by 0x4E4E5A6: sane_dll_get_devices (dll.c:1062)
    ==9049== by 0x4021BA: main (scanimage.c:1973)
    ==9049== Uninitialised value was created by a stack allocation
    ==9049== at 0xA688AD3: ReadConfig (model.c:95)
    ==9049==
    ==9049== Conditional jump or move depends on uninitialised value(s)
    ==9049== at 0xA688A9B: GetPair (model.c:82)
    ==9049== by 0xA688B8A: ReadConfig (model.c:123)
    ==9049== by 0xA6895A2: hpmud_get_model_attributes (model.c:522)
    ==9049== by 0xA6896C8: hpmud_query_model (model.c:563)
    ==9049== by 0xA691CB8: musb_probe_devices (musb.c:2100)
    ==9049== by 0xA68742C: hpmud_probe_devices (hpmud.c:598)
    ==9049== by 0xA23A1A4: DevDiscovery (in /usr/lib/sane/libsane-hpaio.so.1.0.0)
    ==9049== by 0xA23A7A8: sane_hpaio_get_devices (in /usr/lib/sane/libsane-hpaio.so.1.0.0)
    ==9049== by 0x4E4E5A6: sane_dll_get_devices (dll.c:1062)
    ==9049== by 0x4021BA: main (scanimage.c:1973)
    ==9049== Uninitialised value was created by a stack allocation
    ==9049== at 0xA688AD3: ReadConfig (model.c:95)
    ==9049==
    ==9049== Warning: invalid file descriptor -1 in syscall close()
    ==9049== Source and destination overlap in memcpy(0xa984048, 0xa98404a, 155)
    ==9049== at 0x4C2BCC3: memcpy@@GLIBC_2.14 (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==9049== by 0xA69036E: device_id (musb.c:771)
    ==9049== by 0xA691927: musb_open (musb.c:1157)
    ==9049== by 0xA687269: hpmud_open_device (hpmud.c:507)
    ==9049== by 0xA24A719: sclpml_open (in /usr/lib/sane/libsane-hpaio.so.1.0.0)
    ==9049== by 0xA23A8A2: sane_hpaio_open (in /usr/lib/sane/libsane-hpaio.so.1.0.0)
    ==9049== by 0x4E4E9D3: sane_dll_open (dll.c:1204)
    ==9049== by 0x401E58: main (scanimage.c:1989)
    ==9049==
    ==9049== Source and destination overlap in memcpy(0xa984048, 0xa98404a, 155)
    ==9049== at 0x4C2BCC3: memcpy@@GLIBC_2.14 (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==9049== by 0xA69036E: device_id (musb.c:771)
    ==9049== by 0xA69064C: musb_get_device_id (musb.c:1238)
    ==9049== by 0xA68736A: hpmud_get_device_id (hpmud.c:553)
    ==9049== by 0xA24A7BD: sclpml_open (in /usr/lib/sane/libsane-hpaio.so.1.0.0)
    ==9049== by 0xA23A8A2: sane_hpaio_open (in /usr/lib/sane/libsane-hpaio.so.1.0.0)
    ==9049== by 0x4E4E9D3: sane_dll_open (dll.c:1204)
    ==9049== by 0x401E58: main (scanimage.c:1989)
    ==9049==
    ==9049==
    ==9049== HEAP SUMMARY:
    ==9049== in use at exit: 326,378 bytes in 1,273 blocks
    ==9049== total heap usage: 47,198 allocs, 45,925 frees, 4,943,197 bytes allocated
    ==9049==
    ==9049== 21 bytes in 1 blocks are definitely lost in loss record 17 of 155
    ==9049== at 0x4C27730: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==9049== by 0x50E1709: strdup (in /usr/lib/libc-2.18.so)
    ==9049== by 0xA24A856: sclpml_open (in /usr/lib/sane/libsane-hpaio.so.1.0.0)
    ==9049== by 0xA23A8A2: sane_hpaio_open (in /usr/lib/sane/libsane-hpaio.so.1.0.0)
    ==9049== by 0x4E4E9D3: sane_dll_open (dll.c:1204)
    ==9049== by 0x401E58: main (scanimage.c:1989)
    ==9049==
    ==9049== 48 bytes in 1 blocks are definitely lost in loss record 83 of 155
    ==9049== at 0x4C27730: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==9049== by 0x50E1709: strdup (in /usr/lib/libc-2.18.so)
    ==9049== by 0xA24A7FD: sclpml_open (in /usr/lib/sane/libsane-hpaio.so.1.0.0)
    ==9049== by 0xA23A8A2: sane_hpaio_open (in /usr/lib/sane/libsane-hpaio.so.1.0.0)
    ==9049== by 0x4E4E9D3: sane_dll_open (dll.c:1204)
    ==9049== by 0x401E58: main (scanimage.c:1989)
    ==9049==
    ==9049== 154 (144 direct, 10 indirect) bytes in 1 blocks are definitely lost in loss record 122 of 155
    ==9049== at 0x4C27730: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==9049== by 0xA23B022: MfpdtfAllocate (in /usr/lib/sane/libsane-hpaio.so.1.0.0)
    ==9049== by 0xA24BCAC: sclpml_open (in /usr/lib/sane/libsane-hpaio.so.1.0.0)
    ==9049== by 0xA23A8A2: sane_hpaio_open (in /usr/lib/sane/libsane-hpaio.so.1.0.0)
    ==9049== by 0x4E4E9D3: sane_dll_open (dll.c:1204)
    ==9049== by 0x401E58: main (scanimage.c:1989)
    ==9049==
    ==9049== LEAK SUMMARY:
    ==9049== definitely lost: 213 bytes in 3 blocks
    ==9049== indirectly lost: 10 bytes in 1 blocks
    ==9049== possibly lost: 0 bytes in 0 blocks
    ==9049== still reachable: 326,155 bytes in 1,269 blocks
    ==9049== suppressed: 0 bytes in 0 blocks
    ==9049== Reachable blocks (those to which a pointer was found) are not shown.
    ==9049== To see them, rerun with: --leak-check=full --show-leak-kinds=all
    ==9049==
    ==9049== For counts of detected and suppressed errors, rerun with: -v
    ==9049== ERROR SUMMARY: 9 errors from 7 contexts (suppressed: 2 from 2)

As you can see, 213 bytes of leak.

I get into the source code, made a few changes to plug/fix these allocation leak:

diff --git a/scan/sane/sclpml.c b/scan/sane/sclpml.c
index 0ce43bf..05c04bb 100644
--- a/scan/sane/sclpml.c
+++ b/scan/sane/sclpml.c
@@ -2071,6 +2071,10 @@ abort:
             {
                 free( ( void * ) session->saneDevice.model );
             }
+ if (session->mfpdtf)
+ {
+ MfpdtfDeallocate(session->mfpdtf);
+ }
             free( session );
             session = NULL;
         }
@@ -2100,6 +2104,19 @@ void sclpml_close(SANE_Handle handle)
        hpmud_close_device(hpaio->deviceid);
        hpaio->deviceid = -1;
     }
+
+ if(hpaio->saneDevice.name)
+ {
+ free( ( void * ) hpaio->saneDevice.name );
+ }
+ if(hpaio->saneDevice.model)
+ {
+ free( ( void * ) hpaio->saneDevice.model );
+ }
+ if (hpaio->mfpdtf)
+ {
+ MfpdtfDeallocate(hpaio->mfpdtf);
+ }
     free(hpaio);
     session = NULL;
 }

It seems the developers forgot to deallocate some resources especially when
SANE call sane_hpaio_close. This is the result from valgrind after my patch:

    $ valgrind --leak-check=full --leak-resolution=high --track-origins=yes scanimage > /dev/null
    ==9655== Memcheck, a memory error detector
    ==9655== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
    ==9655== Using Valgrind-3.9.0 and LibVEX; rerun with -h for copyright info
    ==9655== Command: scanimage
    ==9655==
    ==9655== Conditional jump or move depends on uninitialised value(s)
    ==9655== at 0xA688A20: GetPair (model.c:73)
    ==9655== by 0xA688B8A: ReadConfig (model.c:123)
    ==9655== by 0xA6895A2: hpmud_get_model_attributes (model.c:522)
    ==9655== by 0xA6896C8: hpmud_query_model (model.c:563)
    ==9655== by 0xA691CB8: musb_probe_devices (musb.c:2100)
    ==9655== by 0xA68742C: hpmud_probe_devices (hpmud.c:598)
    ==9655== by 0xA23A1A4: DevDiscovery (in /usr/lib/sane/libsane-hpaio.so.1.0.0)
    ==9655== by 0xA23A7A8: sane_hpaio_get_devices (in /usr/lib/sane/libsane-hpaio.so.1.0.0)
    ==9655== by 0x4E4E5A6: sane_dll_get_devices (dll.c:1062)
    ==9655== by 0x4021BA: main (scanimage.c:1973)
    ==9655== Uninitialised value was created by a stack allocation
    ==9655== at 0xA688AD3: ReadConfig (model.c:95)
    ==9655==
    ==9655== Conditional jump or move depends on uninitialised value(s)
    ==9655== at 0xA688A9B: GetPair (model.c:82)
    ==9655== by 0xA688B8A: ReadConfig (model.c:123)
    ==9655== by 0xA6895A2: hpmud_get_model_attributes (model.c:522)
    ==9655== by 0xA6896C8: hpmud_query_model (model.c:563)
    ==9655== by 0xA691CB8: musb_probe_devices (musb.c:2100)
    ==9655== by 0xA68742C: hpmud_probe_devices (hpmud.c:598)
    ==9655== by 0xA23A1A4: DevDiscovery (in /usr/lib/sane/libsane-hpaio.so.1.0.0)
    ==9655== by 0xA23A7A8: sane_hpaio_get_devices (in /usr/lib/sane/libsane-hpaio.so.1.0.0)
    ==9655== by 0x4E4E5A6: sane_dll_get_devices (dll.c:1062)
    ==9655== by 0x4021BA: main (scanimage.c:1973)
    ==9655== Uninitialised value was created by a stack allocation
    ==9655== at 0xA688AD3: ReadConfig (model.c:95)
    ==9655==
    ==9655== Warning: invalid file descriptor -1 in syscall close()
    ==9655== Source and destination overlap in memcpy(0xa984048, 0xa98404a, 155)
    ==9655== at 0x4C2BCC3: memcpy@@GLIBC_2.14 (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==9655== by 0xA69036E: device_id (musb.c:771)
    ==9655== by 0xA691927: musb_open (musb.c:1157)
    ==9655== by 0xA687269: hpmud_open_device (hpmud.c:507)
    ==9655== by 0xA24A719: sclpml_open (in /usr/lib/sane/libsane-hpaio.so.1.0.0)
    ==9655== by 0xA23A8A2: sane_hpaio_open (in /usr/lib/sane/libsane-hpaio.so.1.0.0)
    ==9655== by 0x4E4E9D3: sane_dll_open (dll.c:1204)
    ==9655== by 0x401E58: main (scanimage.c:1989)
    ==9655==
    ==9655== Source and destination overlap in memcpy(0xa984048, 0xa98404a, 155)
    ==9655== at 0x4C2BCC3: memcpy@@GLIBC_2.14 (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==9655== by 0xA69036E: device_id (musb.c:771)
    ==9655== by 0xA69064C: musb_get_device_id (musb.c:1238)
    ==9655== by 0xA68736A: hpmud_get_device_id (hpmud.c:553)
    ==9655== by 0xA24A7D9: sclpml_open (in /usr/lib/sane/libsane-hpaio.so.1.0.0)
    ==9655== by 0xA23A8A2: sane_hpaio_open (in /usr/lib/sane/libsane-hpaio.so.1.0.0)
    ==9655== by 0x4E4E9D3: sane_dll_open (dll.c:1204)
    ==9655== by 0x401E58: main (scanimage.c:1989)
    ==9655==
    ==9655==
    ==9655== HEAP SUMMARY:
    ==9655== in use at exit: 326,155 bytes in 1,269 blocks
    ==9655== total heap usage: 58,307 allocs, 57,038 frees, 5,967,093 bytes allocated
    ==9655==
    ==9655== LEAK SUMMARY:
    ==9655== definitely lost: 0 bytes in 0 blocks
    ==9655== indirectly lost: 0 bytes in 0 blocks
    ==9655== possibly lost: 0 bytes in 0 blocks
    ==9655== still reachable: 326,155 bytes in 1,269 blocks
    ==9655== suppressed: 0 bytes in 0 blocks
    ==9655== Reachable blocks (those to which a pointer was found) are not shown.
    ==9655== To see them, rerun with: --leak-check=full --show-leak-kinds=all
    ==9655==
    ==9655== For counts of detected and suppressed errors, rerun with: -v
    ==9655== ERROR SUMMARY: 6 errors from 4 contexts (suppressed: 2 from 2)

I suppose this is a backend issue not SANE, so I report it here.

Thank you.