[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Libcdio-devel] Fedora bug report concerning CD-TEXT
From: |
Thomas Schmitt |
Subject: |
Re: [Libcdio-devel] Fedora bug report concerning CD-TEXT |
Date: |
Sat, 02 Apr 2016 09:23:29 +0200 |
Hi,
it turned out that the code about reading ISRC is potentially dangerous
because its strdup() relies on MMC-4 specs which promise a 0-byte
in byte 17 of the reply of READ SUB-CHANNEL.
Although i would bet CD semantics on that 0-byte, i would not bet
the memory integrity of the program on it.
So the test whether transfer length 24 yields better results
should be done with this change:
--- lib/driver/mmc/mmc.c.orig 2014-09-25 03:19:42.000000000 +0200
+++ lib/driver/mmc/mmc.c 2016-04-02 08:59:01.285553911 +0200
@@ -551,7 +551,7 @@ mmc_get_track_isrc_private ( void *p_env
)
{
mmc_cdb_t cdb = {{0, }};
- char buf[28] = { 0, };
+ char buf[25];
int i_status;
if ( ! p_env || ! run_mmc_cmd )
@@ -559,6 +559,7 @@ mmc_get_track_isrc_private ( void *p_env
CDIO_MMC_SET_COMMAND(cdb.field, CDIO_MMC_GPCMD_READ_SUBCHANNEL);
CDIO_MMC_SET_READ_LENGTH8(cdb.field, sizeof(buf));
+ memset(buf, 0, sizeof(buf));
cdb.field[1] = 0x0;
cdb.field[2] = 0x40;
@@ -568,8 +569,9 @@ mmc_get_track_isrc_private ( void *p_env
i_status = run_mmc_cmd(p_env, mmc_timeout_ms,
mmc_get_cmd_len(cdb.field[0]),
&cdb, SCSI_MMC_DATA_READ,
- sizeof(buf), buf);
+ 24, buf);
if(i_status == 0) {
+ buf[24] = 0;
return strdup(&buf[9]);
}
return NULL;
The use of constants 24 and 25 might not match the coding style of
libcdio. So this is only a rough test proposal, not a patch for
inclusion in libcdio.
This was tested by prepending "valgrind" to the program start in
script ./src/cd-info . Result:
==29605== LEAK SUMMARY:
==29605== definitely lost: 40 bytes in 3 blocks
...
==29605== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
With additional valgrind option --leak-check=full the memory leaks
are reported as:
==31996== 14 bytes in 1 blocks are definitely lost in loss record 2 of 3
==31996== at 0x4C28C20: malloc (vg_replace_malloc.c:296)
==31996== by 0x55EB9D9: strdup (strdup.c:42)
==31996== by 0x504C66D: get_mcn_linux (gnu_linux.c:525)
==31996== by 0x402DEC: main (cd-info.c:1117)
==31996==
==31996== 26 bytes in 2 blocks are definitely lost in loss record 3 of 3
==31996== at 0x4C28C20: malloc (vg_replace_malloc.c:296)
==31996== by 0x55EB9D9: strdup (strdup.c:42)
==31996== by 0x5055F8E: mmc_get_track_isrc_private (mmc.c:586)
==31996== by 0x402E38: main (cd-info.c:1133)
(Stacks are obviously sparser than in source, due to optimization.)
Have a nice day :)
Thomas
- Re: [Libcdio-devel] Fedora bug report concerning CD-TEXT, Thomas Schmitt, 2016/04/01
- Re: [Libcdio-devel] Fedora bug report concerning CD-TEXT, Leon Merten Lohse, 2016/04/01
- Re: [Libcdio-devel] Fedora bug report concerning CD-TEXT, Thomas Schmitt, 2016/04/01
- Re: [Libcdio-devel] Fedora bug report concerning CD-TEXT,
Thomas Schmitt <=
- Re: [Libcdio-devel] Fedora bug report concerning CD-TEXT, Leon Merten Lohse, 2016/04/02
- Re: [Libcdio-devel] Fedora bug report concerning CD-TEXT, Thomas Schmitt, 2016/04/02
- Re: [Libcdio-devel] Fedora bug report concerning CD-TEXT, Rocky Bernstein, 2016/04/02
- Re: [Libcdio-devel] Fedora bug report concerning CD-TEXT, Thomas Schmitt, 2016/04/03
- Re: [Libcdio-devel] Fedora bug report concerning CD-TEXT, Leon Merten Lohse, 2016/04/03
- Re: [Libcdio-devel] Fedora bug report concerning CD-TEXT, Thomas Schmitt, 2016/04/03
- Re: [Libcdio-devel] Fedora bug report concerning CD-TEXT, Thomas Schmitt, 2016/04/03
- Re: [Libcdio-devel] Fedora bug report concerning CD-TEXT, Rocky Bernstein, 2016/04/03
- Re: [Libcdio-devel] Fedora bug report concerning CD-TEXT, Thomas Schmitt, 2016/04/03
- Re: [Libcdio-devel] Fedora bug report concerning CD-TEXT, Rocky Bernstein, 2016/04/03