Opened 17 years ago
Last modified 13 years ago
#751 new defect
NetBSD-specific code in stream/vcd_read_nbsd.h has problems
Reported by: | Owned by: | reimar | |
---|---|---|---|
Priority: | normal | Component: | streaming |
Version: | HEAD | Severity: | normal |
Keywords: | Cc: | ||
Blocked By: | Blocking: | ||
Reproduced by developer: | no | Analyzed by developer: | no |
Description
a) Sector address conversion routines do not take into account
150-sector offset (MSF address 0:0.0 is LBA -150); thus, wrong locations
on disc are read. Usual failure mode: the disc will be played without
video. Surprisingly, some discs will still play correctly.
b) The "error" field of struct scsireq is unused by NetBSD kernel and
may contain random data.
Attachments (2)
Change History (18)
by , 17 years ago
Attachment: | patch-netbsd-vcd added |
---|
comment:1 by , 17 years ago
comment:2 by , 17 years ago
Component: | demuxer → streaming |
---|
Patch forwarded to the mplayer-dev-eng mailinglist.
I have no clue about NetBSD and I can't test the patch.
comment:3 by , 17 years ago
I have posted this patch to devel list earlier, but got no usable response. See http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/2006-October/046670.html.
comment:5 by , 16 years ago
Owner: | changed from | to
---|
The code no longer exists in that form, it has been merged with the FreeBSD code.
The 150 sector offset has been fixed, and error field has never caused problems.
Note that "The "error" field of struct scsireq is unused by NetBSD kernel" and "may contain random data" contradict each other, if it is really unused by the kernel, the kernel would not change it so it will keep its initialization value of 0.
If the kernel fills in random data, I'd be really worried, a kernel should never leak unspecified data to user programs.
comment:6 by , 16 years ago
Resolution: | → worksforme |
---|---|
Status: | new → closed |
The current code works for me, reopen if it causes problems or you have "proof" that it could cause problems.
comment:7 by , 16 years ago
Resolution: | worksforme |
---|---|
Status: | closed → reopened |
New code causes memory corruption (only on NetBSD), because
sc.datalen = 2328 is larger than VCD_SECTOR_DATA (2324).
comment:8 by , 16 years ago
That part of the "new" code was like that always (esp. also long before you submitted your patch).
So does sc.datalen = VCD_SECTOR_DATA work or how is this supposed to be done on NetBSD?
comment:9 by , 16 years ago
Owner: | changed from | to
---|---|
Status: | reopened → new |
It worked before because buffer was larger (VCD_SECTOR_SIZE = 2352). I don't remember why 2328 was required, but will find out.
comment:10 by , 16 years ago
Just FYI, I changed it to sc.datalen = VCD_SECTOR_DATA , if this does not work it must be done differently with an extra buffer and memcpy.
While it did work previously without this, it was still wrong (against the stream API).
comment:11 by , 16 years ago
MMC-3 spec [1] says:
Only sectors (see Table 350) that have a user data field of 2 324 shall be returned. <...> NOTE: 4 spare bytes are included making the total data length returned 2 328 bytes/sector.
So it seems I'd have to use memcpy after all.
comment:12 by , 16 years ago
attachments.isobsolete: | 0 → 1 |
---|
comment:15 by , 15 years ago
The patch in question was included into NetBSD pkgsrc package. Please review it.
comment:16 by , 13 years ago
Owner: | changed from | to
---|
proposed patch