From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1PEovu-00040i-9H for mharc-grub-devel@gnu.org; Sat, 06 Nov 2010 16:00:14 -0400 Received: from [140.186.70.92] (port=56746 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PEovr-00040d-Np for grub-devel@gnu.org; Sat, 06 Nov 2010 16:00:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PEovq-0002OI-9L for grub-devel@gnu.org; Sat, 06 Nov 2010 16:00:11 -0400 Received: from mail-bw0-f41.google.com ([209.85.214.41]:49700) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PEovp-0002O7-Oc for grub-devel@gnu.org; Sat, 06 Nov 2010 16:00:10 -0400 Received: by bwz16 with SMTP id 16so3697355bwz.0 for ; Sat, 06 Nov 2010 13:00:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:message-id:date:from :user-agent:mime-version:to:subject:references:in-reply-to :x-enigmail-version:content-type; bh=5Pezl3C3FxV0y0vhUemjnL0D/b+DRAbH3BCL3EQdcYo=; b=xrFPry0LX501oOMap1g3J4EFJxuQh90tLE86sTuYQl++/n4/Ik73ot530WU+vXCZ3m yxX74/DEkyIphgMfP9KDYTBq1btfTkAgb2+iyJdPNR3CngHHGspTfJNOqydsSQOLmG9b l+qYCO8GIrZHu+Mr7q+q/oGLy3+2Rj5ClcNPI= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:user-agent:mime-version:to:subject:references :in-reply-to:x-enigmail-version:content-type; b=FXjzu4bvbLvwTR6cI7DdVe6+26Pru862ozNDPXocikYuNC4cuOv3QqOeJNcPNHSPfg Vr6yHoS4K9lSnUNaTOt/zZytqyLFq4NolUISRTsJO5ETlt2zuqg82ayvdhqcBijiL1Zn VHjVbZLWu73S1au4pKqLnNWUzafc4GNq77ka4= Received: by 10.204.121.83 with SMTP id g19mr3243015bkr.13.1289073601344; Sat, 06 Nov 2010 13:00:01 -0700 (PDT) Received: from debian.bg45.phnet (245-90.62-81.cust.bluewin.ch [81.62.90.245]) by mx.google.com with ESMTPS id f21sm2213791bkf.12.2010.11.06.12.59.58 (version=TLSv1/SSLv3 cipher=RC4-MD5); Sat, 06 Nov 2010 12:59:59 -0700 (PDT) Message-ID: <4CD5B3B7.1070006@gmail.com> Date: Sat, 06 Nov 2010 20:59:51 +0100 From: =?UTF-8?B?VmxhZGltaXIgJ8+GLWNvZGVyL3BoY29kZXInIFNlcmJpbmVua28=?= User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.15) Gecko/20101030 Icedove/3.0.10 MIME-Version: 1.0 To: grub-devel@gnu.org References: In-Reply-To: X-Enigmail-Version: 1.0.1 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="------------enig7F6F5BCB91CC989C495B9EEB" X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) Subject: Re: UDF filesystem fixes for grub X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 06 Nov 2010 20:00:13 -0000 This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig7F6F5BCB91CC989C495B9EEB Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 11/02/2010 02:10 PM, Giuseppe Caizzone wrote: > Hello, > > I've made 4 small patches that improve the UDF filesystem support in > grub. With these changes, I've been able to read any regular file in > UDF filesystems created by both Linux and Windows on both optical > storage and USB sticks. Perhaps they might be useful for other grub > users? > I don't know if this list is the most appropriate place to post them, > I apologise for the noise if it isn't. > > I'm attaching the patches, which are against last week's trunk, and a > simple bash test case for each. > > - - - - > > 1/4) Support for large (> 2GiB) files > > I know that grub isn't a typical consumer of huge files, but since the > fix is apparently trivial... > > How to test that the patch is needed (tried with grub trunk on linux 2.= 6.36): > > # dd if=3D/dev/zero of=3D/test.bin bs=3D1M count=3D3072 > # mkfs.udf /test.bin > # mount -o loop /test.bin /mnt/m1 > # dd if=3D/dev/urandom of=3D/mnt/m1/bigfile.bin bs=3D1M count=3D2560 > # umount /mnt/m1 > # grub-fstest /test.bin crc "(loop0)/bigfile.bin" > > Result: grub will segfault instead of reading the whole file (tested > on a 32-bit machine). > The patch just changes an int and a grub_uint32_t to be grub_off_t. > > =20 Good patch. Can you write the ChangeLog entry for it? > 2/4) Support for deleted files > > On real-world, non-readonly media, deleted files are likely to appear > inside directories. These files' directory entries have a "deleted" > bit, which grub currently ignores. > > Test case: > > # dd if=3D/dev/zero of=3D/test.bin bs=3D1M count=3D32 > # mkfs.udf /test.bin > # mount -o loop /test.bin /mnt/m1 > # touch /mnt/m1/file{1,2,3,4,5} > # rm /mnt/m1/file3 > # umount /mnt/m1 > # grub-fstest /test.bin ls "(loop0)/" > > Result: only the first two files will be listed, because grub will > "crash" when, encountering the third file's deleted directory entry, > it tries to "dereference" a zeroed field. > The patch just skips directory entries with the "deleted" bit set, in > grub_udf_iterate_dir(). > > =20 This one is good too. Changelog entry? > 3/4) Support for a generic block size > > Currently grub assumes that all UDF filesystems have a logical block > size of 2048 bytes. But the UDF standard states that the block size > should match the physical sector size of the hosting device, so for > instance Windows only creates filesystems with a block size of 512 > bytes on USB sticks. Under Linux, mkudffs allows for other block sizes > as well, up to 4096 bytes. None of these are currently readable by > grub. > > Test case: > > # dd if=3D/dev/zero of=3D/test.bin bs=3D1M count=3D32 > # mkfs.udf --blocksize=3D512 /test.bin > # grub-fstest /test.bin ls "(loop0)/" > > Result: the file system won't be recognised as UDF by grub. > The patch repeats the superblock search in grub_udf_mount() for 512, > 1024, 2048 and 4096 block sizes, stopping if it finds a valid one AND > the sector offset recorded in that superblock matches the detected > superblock offset. So for instance it won't misdetect a superblock > located at sector 256 with blocksize 2048 for a superblock located at > sector 512 with blocksize 1024. > The log2(blocksize/512) is then stored in a new field called "lbshift" > in struct grub_udf_data, which gets used instead of the previous > constant GRUB_UDF_LOG2_BLKSZ, while the constant GRUB_UDF_BLKSZ gets > replaced with the lvd.bsize field already present in struct > grub_udf_data. > > =20 + lbshift =3D 0; + block =3D 0; + while (lbshift <=3D 3) + { For loop is way more appropriate than a while loop + while (lbshift <=3D 3) + { + sblklist =3D sblocklist; + while (*sblklist) + { Same here. ChangeLog entry? > 4/4) Support for allocation descriptor extensions > > When files on an UDF file system grow and fragmentation appears, an > "allocation descriptor extension" entry can be recorded, which points > to a new block containing more allocation descriptors for the file. > Currently grub ignores these entries, and is thus unable to read such > fragmented files properly. > > Test: > > # dd if=3D/dev/zero of=3D/test.bin bs=3D1M count=3D32 > # mkfs.udf --blocksize=3D512 /test.bin > # mount -o loop /test.bin /mnt/m1 > # for ((i=3D0;i<8192;i++)) ; do touch "/mnt/m1/file$i" ; done > # umount /mnt/m1 > # grub-fstest /test.bin ls "(loop0)/" > > Result: only the first 4820 files appear (because the directory itself > gets fragmented, and when grub reaches the jump between the two > fragments, it erroneously tries to read the allocation extension > descriptor as the directory's body). > The patch checks, in grub_udf_read_block(), whether the "allocation > descriptor type", previously ignored, is 3, and in this case, it loads > the referenced block, checks that it's a valid allocation extension > descriptor, and sets the "ad" pointer (and the "len" limit) to > continue the iteration from the allocation descriptors contained in > that block. > > This last one is the only patch which actually contains a proper new > block of code, and it allocates a big structure on the stack (it's a > sector, so it's up to 4K big): I don't know if this is OK, or if > perhaps I should use grub_malloc() instead. Also, the patch depends on > the previous "generic block size" patch. > > =20 + char buf[U32 (node->data->lvd.bsize)]; Will segfault if bsize is too big. You need to allocate on heap - else + else if (U16 (node->fe.tag.tag_ident =3D=3D GRUB_UDF_TAG_IDENT_EFE)) { Parenthesis are wrong. + grub_disk_addr_t sec =3D grub_udf_get_block(node->data, + node->part_ref, + ad->position); ad->position isn't byte-swapped. ChangeLog entry? > Greetings, > -- Giuseppe Caizzone > =20 > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > =20 --=20 Regards Vladimir '=CF=86-coder/phcoder' Serbinenko --------------enig7F6F5BCB91CC989C495B9EEB Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iF4EAREKAAYFAkzVs7cACgkQNak7dOguQgkRWQEAljkH4AeaA9cRmUs4y1kMkCyU UQ1r7DckDivwFk6RCx0A/2Pn9P37K4xJvHria1MFYHYex8WxUrEABcYIPqakVWEj =GGa+ -----END PGP SIGNATURE----- --------------enig7F6F5BCB91CC989C495B9EEB--