bugCompact Disc Input and Control Library - Bugs: bug #22865, crash in udf_readdir() due to...

 
 

bug #22865: crash in udf_readdir() due to i_alloc_descs differences

Submitter:  Vaclav Slavik <vslavik>
Submitted:  Mon 07 Apr 2008 11:29:09 AM UTC
   
 
Category:  run-time error Severity:  5 - Average
Item Group:  None Status:  None
Privacy:  Public Assigned to:  None
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Sun 18 Mar 2012 05:58:22 PM UTC, comment #2: 

I believe recent work from Pete Batard has addressed this. Please try with the current git sources see if there is still a problem.

Thnaks.

Rocky Bernstein <rocky>
Group administrator
Tue 08 Apr 2008 03:27:52 AM UTC, comment #1: 

Thanks for the report and analysis.

Since I don't know what the best thing to do is, sure what you propose sounds "reasonable".

Sorry I can't be of help. This code hasn't gotten a lot of love.

Thanks.

Rocky Bernstein <rocky>
Group administrator
Mon 07 Apr 2008 11:29:09 AM UTC, original submission:  


I'm getting crashes in udf_readdir() using today's CVS sources (present in 0.80 too). Valgrind spits out many entries like this:


==21433== Invalid write of size 1
==21433==    at 0x4A08DAB: memcpy (in /usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)
==21433==    by 0x4C0EFD3: udf_readdir (udf_fs.c:665)
==21433==    by 0x400F49: list_files (udf1.c:75)
==21433==    by 0x4010ED: main (udf1.c:138)
==21433==  Address 0x50c25e8 is 0 bytes after a block of size 304 alloc'd
==21433==    at 0x4A060DB: calloc (in /usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)
==21433==    by 0x4C0E3F4: udf_new_dirent (udf_fs.c:293)
==21433==    by 0x4C0EB5A: udf_get_root (udf_fs.c:550)
==21433==    by 0x400FF2: main (udf1.c:114)


(with the write address going to "1,2,... bytes after" for the same filesystem entry and than again with other files (but not all).

I tried to fix it, but I don't understand the code enough to know  how best to do it. The offending line of code in udf_readdir() is this:


        memcpy(&(p_udf_dirent->fe), p_udf_fe,
               sizeof(udf_file_entry_t) + p_udf_fe->i_alloc_descs
               + p_udf_fe->i_extended_attr );


And when I add asserts to check i_alloc_descs and i_extended_attr  values there, it becomes clear it's because these values differ between p_udf_dirent->fe and p_udf_fe. But this piece of code assumes it is constant, resulting in memory overwrites. (BTW, there's a related FIXME in udf_fopen()).

The only thing I can think of is changing udf_dirent_t.fe to be a pointer and realloc() it at this place if the size differs. Does that sound reasonable? If it does, I may give it a try.

How to reproduce: download VS2008MSDNLibraryENUX1429217.iso from http://www.microsoft.com/downloads/details.aspx?FamilyId=6FF3BC60-32C8-4C22-8591-A20BF8DFF1A2&displaylang=en and run examples/utf1 with this file as its argument. (Sorry, I'm not sure how to create UDF image with the same properties -- this download is 2GB in size.)

Vaclav Slavik <vslavik>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rocky (Posted a comment)
  • -email is unavailable- added by vslavik (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follows 1 latest change.

    Date Changed by Updated Field Previous Value => Replaced by
    2012-04-24 rocky Open/ClosedOpen Closed

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code