Bug 39062

Summary:	drivers/cdrom: dvd_read_manufact() returns Input/output error
Product:	IO/Storage	Reporter:	ale.goujon
Component:	Other	Assignee:	io_other
Status:	RESOLVED CODE_FIX
Severity:	normal	CC:	akpm, axboe
Priority:	P1
Hardware:	All
OS:	Linux
Kernel Version:	2.6.39	Subsystem:
Regression:	No	Bisected commit-id:
Attachments:	A simple testcase drivers-cdrom-cdromc-relax-check-on-dvd-manufacturer-value.patch

Description ale.goujon 2011-07-09 14:12:16 UTC

Created attachment 65092 [details]
A simple testcase

It all started when I tried to play "The Sims 3" game with wine : it was complaining the DVD was not present.
I looked for a wine bug and found it [1].

Another bug [2] has been created with a test case.
The cdrom driver of the kernel has been identified as the culprit.
I'm opening this (upstream) bug following the first comment of [2].

The issue is (as described in the title) the dvd_read_manufact() function returning EIO because of the manufact.len is > 2048.

I'm not a UDF expert but as UDF is a standard, I tried reading the UDF specifications [3] to find a clue that Linux is mis-behaving.
I only found the following sentence which may indicate why the length is > 2048
"These requirements do not apply to the discs that are used in a computer system environment only and have no interchangeability with consumer appliances."
[3], page 147, "6.12 Requirements for DVD-R/-RW/-RAM interchangeability" 

I can easily reproduce the bug by :
1) mounting the UDF ISO :
$cdemu load any ~/Games/TheSims3.iso
$cdemu status
$cdemu-device-mapping
At this point, I know /dev/sr1 is the bloc device associated with my ISO file
2) running the (attached) test case (slightly modified from the bug report [2] to take the /dev/xxx as parameter)
3) gcc testcase.c -o testcase && ./testcase /dev/sr1
stderr :
DVD_READ_STRUCT: Input/output error
or $dmesg | tail
[10755.135612] cdrom: Received invalid manufacture info length (2050)

I personally created the ISO of the game with dd and I mounted it properly prior to running the game through wine.
The ISO file is 5.6GB long so I can't attach it here (and would be illegal).
Maybe I can dd the first n bytes of the ISO file and attach it here but you have to tell me how many bytes you wants.

This is not a regression but if it matters, the
"if (s->manufact.len < 0 || s->manufact.len > 2048) {"
line has been introduced in commit 1da77e [4].

A quick'n'dirty fix is to change the 2048 value to something superior but I know that Linus hates modifying values that worked for years (and he's right).

If you need anything, just tell me what and how.
Thanks in advance
---
[1]: http://bugs.winehq.org/show_bug.cgi?id=26273
[2]: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/777528
[3]: http://www.osta.org/specs/pdf/udf260.pdf
[4]: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=1da177e4c3f41524e886b7f1b8a0c1fc7321cac2

Comment 1 Andrew Morton 2011-07-12 22:53:01 UTC

We can't just increase the 2048 - that would cause the memcpy to overflow struct dvd_manufact and scribble on memory.

I don't think we even use dvd_manufact for anything.  I did a patch to truncate the field to 2048 bytes.  Please test it?

Comment 2 Andrew Morton 2011-07-12 22:54:42 UTC

Created attachment 65362 [details]
drivers-cdrom-cdromc-relax-check-on-dvd-manufacturer-value.patch

the patch

Comment 3 ale.goujon 2011-07-13 11:35:27 UTC

I applied the patch against linux-2.6.39.3 (the last stable release).

Now, when I run the testcase, I get (as expected)
Got len 2048, value '     '
(with value = 2048 spaces )
and in dmesg
[  167.425005] cdrom: Received invalid manufacture info length (2050): truncating to 2048

So the patch works, yay !
I don't know if it can be included for the soon-to-be-released 3.0 linux kernel but whatever, I will ask Ubuntu kernel maintainers if they can backport this minor change.

Now, I will try to get wine fixed.

Thanks for your work guys, it's always a pleasure to run a linux-based OS.

NB: I didn't change the status of this bug as the patch is not yet merged.

Comment 4 ale.goujon 2011-08-15 06:19:25 UTC

Any update on this patch ?

I received an e-mail saying it was merged and so removed from the -mm-tree but if I check latest linus tree [1], the code is still the same.

Can someone tell me on what tree the patch was applied ?
Is there any chance it will be merged in the final 3.1 kernel ?

Thanks
---
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=drivers/cdrom/cdrom.c;hb=HEAD#l1932

Comment 5 ale.goujon 2011-08-20 08:07:12 UTC

I just checked and the patch has been committed in linus tree [1] apparently on Aug. 2 and so will be in the final 3.1 kernel.

I can now mark this bug as resolved.
Yay !

Thank you guys.
---
[1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=aec9f377e4f235c47e27fd8a429555dfa2dda342