grub-devel.gnu.org archive mirror
 help / color / mirror / Atom feed
From: Giuseppe Caizzone <acaizzo@gmail.com>
To: The development of GNU GRUB <grub-devel@gnu.org>
Subject: Re: UDF filesystem fixes for grub
Date: Thu, 11 Nov 2010 09:56:29 +0100	[thread overview]
Message-ID: <AANLkTikbwN98s4dw_6zaFLdHA80e44Ri1TxhqFDsT78n@mail.gmail.com> (raw)
In-Reply-To: <4CD5B3B7.1070006@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 6368 bytes --]

2010/11/6 Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com>:
> 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.
>> [...]
>> 1/4) Support for large (> 2GiB) files
>> [...]
>> The patch just changes an int and a grub_uint32_t to be grub_off_t.
>>
>>
> Good patch. Can you write the ChangeLog entry for it?

OK - I've never written one, so I tried to mimic grub's.

	Support reading files larger than 2 GiB.

	* grub-core/fs/udf.c (grub_udf_iterate_dir): change type of variable
	"offset" from grub_uint32_t to grub_off_t.
	(grub_udf_read_file): change type of parameter "pos" from int to
	grub_off_t.

>> 2/4) Support for deleted files
>> [...]
>> The patch just skips directory entries with the "deleted" bit set, in
>> grub_udf_iterate_dir().
>>
>>
> This one is good too. Changelog entry?

	Properly handle deleted files.

	* grub-core/fs/udf.c (grub_udf_iterate_dir): Skip directory entries
	whose "characteristics" field has the bit GRUB_UDF_FID_CHAR_DELETED
	set.

>> 3/4) Support for a generic block size
>> [...]
>> 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.
>>
>>
> +  lbshift = 0;
> +  block = 0;
> +  while (lbshift <= 3)
> +    {
> For loop is way more appropriate than a while loop

Changed it into a for loop.

> +  while (lbshift <= 3)
> +    {
> +      sblklist = sblocklist;
> +      while (*sblklist)
> +        {
> Same here.

Changed that too.

I also made another change since the previous version of the patch: I
moved the VRS check after the AVDP search, because it needs to know
the logical block size. I originally thought that it wasn't necessary,
because the VRS is made up of records with a fixed size of 2048 bytes,
but the catch is that the standard says that each record must start on
a new sector, so if block size is > 2048, one has to skip more bytes
than 2048 to get to the next record. Tested it with mkudffs because I
have no medium with an actual block size of 4096.

> ChangeLog entry?

	Add generic logical block size support.

	* grub-core/fs/udf.c (GRUB_UDF_LOG2_BLKSIZE): Removed macro.
	(GRUB_UDF_BLKSZ): Removed macro.
	(struct grub_udf_data): New field "lbshift" to hold the logical	block
	size of the file system in log2 format.
	(grub_udf_read_icb): Replace usage of macro GRUB_UDF_LOG2_BLKSZ with
	field lbshift from struct grub_udf_data.
	(grub_udf_read_file): Likewise.
	(grub_udf_read_block): Replace usage of macro GRUB_UDF_BLKSZ with
	field "lvd.bsize" from struct grub_udf_data.
	Replace division with right shift.
	(sblocklist): Change type to unsigned.
	(grub_udf_mount): Change type of "sblklist" to unsigned.
	New variable "vblock" to be used during VRS recognition.
	New variable "lbshift" to hold the detected logical block size.
	Move AVDP search before VRS recognition, because the latter requires
	knowledge of the logical block size, which is detected during the
	former.
	Detect and validate logical block size during AVDP search, adding
	support for block sizes 512, 1024 and 4096.
	Make VRS recognition independent of block size.
	Replace usages of macro GRUB_UDF_LOG2_BLKSZ with variable lbshift.

>> 4/4) Support for allocation descriptor extensions
>> [...]
>> 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.
>>
>>
> +  char buf[U32 (node->data->lvd.bsize)];
> Will segfault if bsize is too big. You need to allocate on heap

Done.

> -  else
> +  else if (U16 (node->fe.tag.tag_ident == GRUB_UDF_TAG_IDENT_EFE))
>     {
> Parenthesis are wrong.

While I was at it, I changed the 'if-else if' to a switch.

> +             grub_disk_addr_t sec = grub_udf_get_block(node->data,
> +                                                       node->part_ref,
> +                                                       ad->position);
> ad->position isn't byte-swapped.

grub_udf_get_block() byte-swaps it itself, so I must not byte-swap it before.

> ChangeLog entry?

	Add support for allocation extent descriptors, needed on fragmented
	volumes.

	* grub-core/fs/udf.c (grub_udf_aed): New struct.
	(grub_udf_read_block): Change type of variable "len" to grub_ssize_t.
	Add sanity check for file entry type.
	Replace divisions with right shifts.
	Check the upper bits of the allocation descriptor's length and honour
	them by loading an allocation extent descriptor if they indicate so.
	Change all failure return paths to free the allocation extent
	descriptor if it was allocated.

I'm attaching the full patch set (including the unchanged ones for
convenience), and also the change log entries in attachment form in
case gmail tampers with whitespace.

Thank you,
-- 
Giuseppe Caizzone <acaizzo@gmail.com>

[-- Attachment #2: 1-udf-large-files.patch --]
[-- Type: application/octet-stream, Size: 731 bytes --]

diff -Naur grub/grub-core/fs/udf.c grub-test/grub-core/fs/udf.c
--- grub/grub-core/fs/udf.c	2010-05-06 17:30:36.000000000 +0200
+++ grub-test/grub-core/fs/udf.c	2010-10-27 10:18:08.160000033 +0200
@@ -471,7 +471,7 @@
 		    void NESTED_FUNC_ATTR
 		    (*read_hook) (grub_disk_addr_t sector,
 				  unsigned offset, unsigned length),
-		    int pos, grub_size_t len, char *buf)
+		    grub_off_t pos, grub_size_t len, char *buf)
 {
   switch (U16 (node->fe.icbtag.flags) & GRUB_UDF_ICBTAG_FLAG_AD_MASK)
     {
@@ -704,7 +704,7 @@
 {
   grub_fshelp_node_t child;
   struct grub_udf_file_ident dirent;
-  grub_uint32_t offset = 0;
+  grub_off_t offset = 0;
 
   child = grub_malloc (sizeof (struct grub_fshelp_node));
   if (!child)

[-- Attachment #3: 1-udf-large-files.changelog --]
[-- Type: application/octet-stream, Size: 236 bytes --]

	Support reading files larger than 2 GiB.

	* grub-core/fs/udf.c (grub_udf_iterate_dir): change type of variable
	"offset" from grub_uint32_t to grub_off_t.
	(grub_udf_read_file): change type of parameter "pos" from int to
	grub_off_t.

[-- Attachment #4: 2-udf-deleted-files.patch --]
[-- Type: application/octet-stream, Size: 3228 bytes --]

diff -Naur grub/grub-core/fs/udf.c grub-test/grub-core/fs/udf.c
--- grub/grub-core/fs/udf.c	2010-05-06 17:30:36.000000000 +0200
+++ grub-test/grub-core/fs/udf.c	2010-10-27 10:19:27.728000018 +0200
@@ -729,57 +729,60 @@
 	  return 0;
 	}
 
-      child = grub_malloc (sizeof (struct grub_fshelp_node));
-      if (!child)
-	return 0;
-
-      if (grub_udf_read_icb (dir->data, &dirent.icb, child))
-	return 0;
-
       offset += sizeof (dirent) + U16 (dirent.imp_use_length);
-      if (dirent.characteristics & GRUB_UDF_FID_CHAR_PARENT)
-	{
-	  /* This is the parent directory.  */
-	  if (hook ("..", GRUB_FSHELP_DIR, child))
-	    return 1;
-	}
-      else
+      if (!(dirent.characteristics & GRUB_UDF_FID_CHAR_DELETED))
 	{
-	  enum grub_fshelp_filetype type;
-	  grub_uint8_t raw[dirent.file_ident_length];
-	  grub_uint16_t utf16[dirent.file_ident_length - 1];
-	  grub_uint8_t filename[dirent.file_ident_length * 2];
-	  grub_size_t utf16len = 0;
-
-	  type = ((dirent.characteristics & GRUB_UDF_FID_CHAR_DIRECTORY) ?
-		  (GRUB_FSHELP_DIR) : (GRUB_FSHELP_REG));
-
-	  if ((grub_udf_read_file (dir, 0, offset,
-				   dirent.file_ident_length,
-				   (char *) raw))
-	      != dirent.file_ident_length)
+	  child = grub_malloc (sizeof (struct grub_fshelp_node));
+	  if (!child)
 	    return 0;
 
-	  if (raw[0] == 8)
-	    {
-	      unsigned i;
-	      utf16len = dirent.file_ident_length - 1;
-	      for (i = 0; i < utf16len; i++)
-		utf16[i] = raw[i + 1];
-	    }
-	  if (raw[0] == 16)
+          if (grub_udf_read_icb (dir->data, &dirent.icb, child))
+	    return 0;
+
+          if (dirent.characteristics & GRUB_UDF_FID_CHAR_PARENT)
 	    {
-	      unsigned i;
-	      utf16len = (dirent.file_ident_length - 1) / 2;
-	      for (i = 0; i < utf16len; i++)
-		utf16[i] = (raw[2 * i + 1] << 8) | raw[2*i + 2];
+	      /* This is the parent directory.  */
+	      if (hook ("..", GRUB_FSHELP_DIR, child))
+	        return 1;
 	    }
-	  if (raw[0] == 8 || raw[0] == 16)
+          else
 	    {
-	      *grub_utf16_to_utf8 (filename, utf16, utf16len) = '\0';
-	  
-	      if (hook ((char *) filename, type, child))
-		return 1;
+	      enum grub_fshelp_filetype type;
+	      grub_uint8_t raw[dirent.file_ident_length];
+	      grub_uint16_t utf16[dirent.file_ident_length - 1];
+	      grub_uint8_t filename[dirent.file_ident_length * 2];
+	      grub_size_t utf16len = 0;
+
+	      type = ((dirent.characteristics & GRUB_UDF_FID_CHAR_DIRECTORY) ?
+		      (GRUB_FSHELP_DIR) : (GRUB_FSHELP_REG));
+
+	      if ((grub_udf_read_file (dir, 0, offset,
+				       dirent.file_ident_length,
+				       (char *) raw))
+		  != dirent.file_ident_length)
+		return 0;
+
+	      if (raw[0] == 8)
+		{
+		  unsigned i;
+		  utf16len = dirent.file_ident_length - 1;
+		  for (i = 0; i < utf16len; i++)
+		    utf16[i] = raw[i + 1];
+		}
+	      if (raw[0] == 16)
+		{
+		  unsigned i;
+		  utf16len = (dirent.file_ident_length - 1) / 2;
+		  for (i = 0; i < utf16len; i++)
+		    utf16[i] = (raw[2 * i + 1] << 8) | raw[2*i + 2];
+		}
+	      if (raw[0] == 8 || raw[0] == 16)
+		{
+		  *grub_utf16_to_utf8 (filename, utf16, utf16len) = '\0';
+
+		  if (hook ((char *) filename, type, child))
+		    return 1;
+		}
 	    }
 	}
 

[-- Attachment #5: 2-udf-deleted-files.changelog --]
[-- Type: application/octet-stream, Size: 177 bytes --]

	Properly handle deleted files.

	* grub-core/fs/udf.c (grub_udf_iterate_dir): Skip directory entries
	whose "characteristics" field has the bit GRUB_UDF_FID_CHAR_DELETED
	set.

[-- Attachment #6: 3-udf-generic-block-size-v2.patch --]
[-- Type: application/octet-stream, Size: 6299 bytes --]

--- grub-old/grub-core/fs/udf.c	2010-11-07 09:19:09.732728000 +0100
+++ grub-new/grub-core/fs/udf.c	2010-11-08 14:30:01.588347621 +0100
@@ -34,9 +34,6 @@
 #define U32				grub_le_to_cpu32
 #define U64				grub_le_to_cpu64
 
-#define GRUB_UDF_LOG2_BLKSZ		2
-#define GRUB_UDF_BLKSZ			2048
-
 #define GRUB_UDF_TAG_IDENT_PVD		0x0001
 #define GRUB_UDF_TAG_IDENT_AVDP		0x0002
 #define GRUB_UDF_TAG_IDENT_VDP		0x0003
@@ -343,7 +340,7 @@
   struct grub_udf_pd pds[GRUB_UDF_MAX_PDS];
   struct grub_udf_partmap *pms[GRUB_UDF_MAX_PMS];
   struct grub_udf_long_ad root_icb;
-  int npd, npm;
+  int npd, npm, lbshift;
 };
 
 struct grub_fshelp_node
@@ -389,7 +386,7 @@
   if (grub_errno)
     return grub_errno;
 
-  if (grub_disk_read (data->disk, block << GRUB_UDF_LOG2_BLKSZ, 0,
+  if (grub_disk_read (data->disk, block << data->lbshift, 0,
 		      sizeof (struct grub_udf_file_entry),
 		      &node->fe))
     return grub_errno;
@@ -427,7 +424,7 @@
       struct grub_udf_short_ad *ad = (struct grub_udf_short_ad *) ptr;
 
       len /= sizeof (struct grub_udf_short_ad);
-      filebytes = fileblock * GRUB_UDF_BLKSZ;
+      filebytes = fileblock * U32 (node->data->lvd.bsize);
       while (len > 0)
 	{
 	  if (filebytes < U32 (ad->length))
@@ -435,7 +432,8 @@
                     (grub_udf_get_block (node->data,
                                          node->part_ref,
                                          ad->position)
-                     + (filebytes / GRUB_UDF_BLKSZ)));
+                     + (filebytes >> (GRUB_DISK_SECTOR_BITS
+				      + node->data->lbshift))));
 
 	  filebytes -= U32 (ad->length);
 	  ad++;
@@ -447,7 +445,7 @@
       struct grub_udf_long_ad *ad = (struct grub_udf_long_ad *) ptr;
 
       len /= sizeof (struct grub_udf_long_ad);
-      filebytes = fileblock * GRUB_UDF_BLKSZ;
+      filebytes = fileblock * U32 (node->data->lvd.bsize);
       while (len > 0)
 	{
 	  if (filebytes < U32 (ad->length))
@@ -455,7 +453,8 @@
                     (grub_udf_get_block (node->data,
                                          ad->block.part_ref,
                                          ad->block.block_num)
-		     + (filebytes / GRUB_UDF_BLKSZ)));
+		     + (filebytes >> (GRUB_DISK_SECTOR_BITS
+				      + node->data->lbshift))));
 
 	  filebytes -= U32 (ad->length);
 	  ad++;
@@ -496,21 +495,21 @@
     }
 
   return  grub_fshelp_read_file (node->data->disk, node, read_hook,
-                                 pos, len, buf, grub_udf_read_block,
-                                 U64 (node->fe.file_size),
-                                 GRUB_UDF_LOG2_BLKSZ);
+				 pos, len, buf, grub_udf_read_block,
+				 U64 (node->fe.file_size),
+				 node->data->lbshift);
 }
 
-static int sblocklist[] = { 256, 512, 0 };
+static unsigned sblocklist[] = { 256, 512, 0 };
 
 static struct grub_udf_data *
 grub_udf_mount (grub_disk_t disk)
 {
   struct grub_udf_data *data = 0;
   struct grub_udf_fileset root_fs;
-  int *sblklist = sblocklist;
-  grub_uint32_t block;
-  int i;
+  unsigned *sblklist;
+  grub_uint32_t block, vblock;
+  int i, lbshift;
 
   data = grub_malloc (sizeof (struct grub_udf_data));
   if (!data)
@@ -518,12 +517,48 @@
 
   data->disk = disk;
 
+  /* Search for Anchor Volume Descriptor Pointer (AVDP)
+   * and determine logical block size.  */
+  block = 0;
+  for (lbshift = 0; lbshift < 4; lbshift++)
+    {
+      for (sblklist = sblocklist; *sblklist; sblklist++)
+        {
+	  struct grub_udf_avdp avdp;
+
+	  if (grub_disk_read (disk, *sblklist << lbshift, 0,
+			      sizeof (struct grub_udf_avdp), &avdp))
+	    {
+	      grub_error (GRUB_ERR_BAD_FS, "not an UDF filesystem");
+	      goto fail;
+	    }
+
+	  if (U16 (avdp.tag.tag_ident) == GRUB_UDF_TAG_IDENT_AVDP &&
+	      U32 (avdp.tag.tag_location) == *sblklist)
+	    {
+	      block = U32 (avdp.vds.start);
+	      break;
+	    }
+	}
+
+      if (block)
+	break;
+    }
+
+  if (!block)
+    {
+      grub_error (GRUB_ERR_BAD_FS, "not an UDF filesystem");
+      goto fail;
+    }
+  data->lbshift = lbshift;
+
   /* Search for Volume Recognition Sequence (VRS).  */
-  for (block = 16;; block++)
+  for (vblock = (32767 >> (lbshift + GRUB_DISK_SECTOR_BITS)) + 1;;
+       vblock += (2047 >> (lbshift + GRUB_DISK_SECTOR_BITS)) + 1)
     {
       struct grub_udf_vrs vrs;
 
-      if (grub_disk_read (disk, block << GRUB_UDF_LOG2_BLKSZ, 0,
+      if (grub_disk_read (disk, vblock << lbshift, 0,
 			  sizeof (struct grub_udf_vrs), &vrs))
 	{
 	  grub_error (GRUB_ERR_BAD_FS, "not an UDF filesystem");
@@ -545,39 +580,13 @@
 	}
     }
 
-  /* Search for Anchor Volume Descriptor Pointer (AVDP).  */
-  while (1)
-    {
-      struct grub_udf_avdp avdp;
-
-      if (grub_disk_read (disk, *sblklist << GRUB_UDF_LOG2_BLKSZ, 0,
-			  sizeof (struct grub_udf_avdp), &avdp))
-	{
-	  grub_error (GRUB_ERR_BAD_FS, "not an UDF filesystem");
-	  goto fail;
-	}
-
-      if (U16 (avdp.tag.tag_ident) == GRUB_UDF_TAG_IDENT_AVDP)
-	{
-	  block = U32 (avdp.vds.start);
-	  break;
-	}
-
-      sblklist++;
-      if (*sblklist == 0)
-	{
-	  grub_error (GRUB_ERR_BAD_FS, "not an UDF filesystem");
-	  goto fail;
-	}
-    }
-
   data->npd = data->npm = 0;
   /* Locate Partition Descriptor (PD) and Logical Volume Descriptor (LVD).  */
   while (1)
     {
       struct grub_udf_tag tag;
 
-      if (grub_disk_read (disk, block << GRUB_UDF_LOG2_BLKSZ, 0,
+      if (grub_disk_read (disk, block << lbshift, 0,
 			  sizeof (struct grub_udf_tag), &tag))
 	{
 	  grub_error (GRUB_ERR_BAD_FS, "not an UDF filesystem");
@@ -593,7 +602,7 @@
 	      goto fail;
 	    }
 
-	  if (grub_disk_read (disk, block << GRUB_UDF_LOG2_BLKSZ, 0,
+	  if (grub_disk_read (disk, block << lbshift, 0,
 			      sizeof (struct grub_udf_pd),
 			      &data->pds[data->npd]))
 	    {
@@ -609,7 +618,7 @@
 
 	  struct grub_udf_partmap *ppm;
 
-	  if (grub_disk_read (disk, block << GRUB_UDF_LOG2_BLKSZ, 0,
+	  if (grub_disk_read (disk, block << lbshift, 0,
 			      sizeof (struct grub_udf_lvd),
 			      &data->lvd))
 	    {
@@ -673,7 +682,7 @@
   if (grub_errno)
     goto fail;
 
-  if (grub_disk_read (disk, block << GRUB_UDF_LOG2_BLKSZ, 0,
+  if (grub_disk_read (disk, block << lbshift, 0,
 		      sizeof (struct grub_udf_fileset), &root_fs))
     {
       grub_error (GRUB_ERR_BAD_FS, "not an UDF filesystem");

[-- Attachment #7: 3-udf-generic-block-size-v2.changelog --]
[-- Type: application/octet-stream, Size: 1140 bytes --]

	Add generic logical block size support.

	* grub-core/fs/udf.c (GRUB_UDF_LOG2_BLKSIZE): Removed macro.
	(GRUB_UDF_BLKSZ): Removed macro.
	(struct grub_udf_data): New field "lbshift" to hold the logical	block
	size of the file system in log2 format.
	(grub_udf_read_icb): Replace usage of macro GRUB_UDF_LOG2_BLKSZ with
	field lbshift from struct grub_udf_data.
	(grub_udf_read_file): Likewise.
	(grub_udf_read_block): Replace usage of macro GRUB_UDF_BLKSZ with
	field "lvd.bsize" from struct grub_udf_data.
	Replace division with right shift.
	(sblocklist): Change type to unsigned.
	(grub_udf_mount): Change type of "sblklist" to unsigned.
	New variable "vblock" to be used during VRS recognition.
	New variable "lbshift" to hold the detected logical block size.
	Move AVDP search before VRS recognition, because the latter requires
	knowledge of the logical block size, which is detected during the
	former.
	Detect and validate logical block size during AVDP search, adding
	support for block sizes 512, 1024 and 4096.
	Make VRS recognition independent of block size.
	Replace usages of macro GRUB_UDF_LOG2_BLKSZ with variable lbshift.

[-- Attachment #8: 4-udf-ad-extents-v2.patch --]
[-- Type: application/octet-stream, Size: 5511 bytes --]

diff -Naur grub-old/grub-core/fs/udf.c grub-new/grub-core/fs/udf.c
--- grub-old/grub-core/fs/udf.c	2010-11-09 09:23:53.713106502 +0100
+++ grub-new/grub-core/fs/udf.c	2010-11-09 09:21:08.351056468 +0100
@@ -333,6 +333,13 @@
   grub_uint8_t part_maps[1608];
 } __attribute__ ((packed));
 
+struct grub_udf_aed
+{
+  struct grub_udf_tag tag;
+  grub_uint32_t prev_ae;
+  grub_uint32_t ae_len;
+} __attribute__ ((packed));
+
 struct grub_udf_data
 {
   grub_disk_t disk;
@@ -403,19 +410,26 @@
 static grub_disk_addr_t
 grub_udf_read_block (grub_fshelp_node_t node, grub_disk_addr_t fileblock)
 {
+  char *buf = NULL;
   char *ptr;
-  int len;
+  grub_ssize_t len;
   grub_disk_addr_t filebytes;
 
-  if (U16 (node->fe.tag.tag_ident) == GRUB_UDF_TAG_IDENT_FE)
+  switch (U16 (node->fe.tag.tag_ident))
     {
+    case GRUB_UDF_TAG_IDENT_FE:
       ptr = (char *) &node->fe.ext_attr[0] + U32 (node->fe.ext_attr_length);
       len = U32 (node->fe.alloc_descs_length);
-    }
-  else
-    {
+      break;
+
+    case GRUB_UDF_TAG_IDENT_EFE:
       ptr = (char *) &node->efe.ext_attr[0] + U32 (node->efe.ext_attr_length);
       len = U32 (node->efe.alloc_descs_length);
+      break;
+
+    default:
+      grub_error (GRUB_ERR_BAD_FS, "invalid file entry");
+      return 0;
     }
 
   if ((U16 (node->fe.icbtag.flags) & GRUB_UDF_ICBTAG_FLAG_AD_MASK)
@@ -423,45 +437,115 @@
     {
       struct grub_udf_short_ad *ad = (struct grub_udf_short_ad *) ptr;
 
-      len /= sizeof (struct grub_udf_short_ad);
       filebytes = fileblock * U32 (node->data->lvd.bsize);
-      while (len > 0)
+      while (len >= (grub_ssize_t) sizeof (struct grub_udf_short_ad))
 	{
-	  if (filebytes < U32 (ad->length))
-	    return ((U32 (ad->position) & GRUB_UDF_EXT_MASK) ? 0 :
-                    (grub_udf_get_block (node->data,
-                                         node->part_ref,
-                                         ad->position)
-                     + (filebytes >> (GRUB_DISK_SECTOR_BITS
-				      + node->data->lbshift))));
+	  grub_uint32_t adlen = U32 (ad->length) & 0x3fffffff;
+	  grub_uint32_t adtype = U32 (ad->length) >> 30;
+	  if (adtype == 3)
+	    {
+	      struct grub_udf_aed *extension;
+	      grub_disk_addr_t sec = grub_udf_get_block(node->data,
+							node->part_ref,
+							ad->position);
+	      if (!buf)
+		{
+		  buf = grub_malloc (U32 (node->data->lvd.bsize));
+		  if (!buf)
+		    return 0;
+		}
+	      if (grub_disk_read (node->data->disk, sec << node->data->lbshift,
+				  0, adlen, buf))
+		goto fail;
+
+	      extension = (struct grub_udf_aed *) buf;
+	      if (U16 (extension->tag.tag_ident) != GRUB_UDF_TAG_IDENT_AED)
+		{
+		  grub_error (GRUB_ERR_BAD_FS, "invalid aed tag");
+		  goto fail;
+		}
+
+	      len = U32 (extension->ae_len);
+	      ad = (struct grub_udf_short_ad *)
+		    (buf + sizeof (struct grub_udf_aed));
+	      continue;
+	    }
+
+	  if (filebytes < adlen)
+	    {
+	      grub_uint32_t ad_pos = ad->position;
+	      grub_free (buf);
+	      return ((U32 (ad_pos) & GRUB_UDF_EXT_MASK) ? 0 :
+		      (grub_udf_get_block (node->data, node->part_ref, ad_pos)
+		       + (filebytes >> (GRUB_DISK_SECTOR_BITS
+					+ node->data->lbshift))));
+	    }
 
-	  filebytes -= U32 (ad->length);
+	  filebytes -= adlen;
 	  ad++;
-	  len--;
+	  len -= sizeof (struct grub_udf_short_ad);
 	}
     }
   else
     {
       struct grub_udf_long_ad *ad = (struct grub_udf_long_ad *) ptr;
 
-      len /= sizeof (struct grub_udf_long_ad);
       filebytes = fileblock * U32 (node->data->lvd.bsize);
-      while (len > 0)
+      while (len >= (grub_ssize_t) sizeof (struct grub_udf_long_ad))
 	{
-	  if (filebytes < U32 (ad->length))
-	    return ((U32 (ad->block.block_num) & GRUB_UDF_EXT_MASK) ?  0 :
-                    (grub_udf_get_block (node->data,
-                                         ad->block.part_ref,
-                                         ad->block.block_num)
-		     + (filebytes >> (GRUB_DISK_SECTOR_BITS
-				      + node->data->lbshift))));
+	  grub_uint32_t adlen = U32 (ad->length) & 0x3fffffff;
+	  grub_uint32_t adtype = U32 (ad->length) >> 30;
+	  if (adtype == 3)
+	    {
+	      struct grub_udf_aed *extension;
+	      grub_disk_addr_t sec = grub_udf_get_block(node->data,
+							ad->block.part_ref,
+							ad->block.block_num);
+	      if (!buf)
+		{
+		  buf = grub_malloc (U32 (node->data->lvd.bsize));
+		  if (!buf)
+		    return 0;
+		}
+	      if (grub_disk_read (node->data->disk, sec << node->data->lbshift,
+				  0, adlen, buf))
+		goto fail;
+
+	      extension = (struct grub_udf_aed *) buf;
+	      if (U16 (extension->tag.tag_ident) != GRUB_UDF_TAG_IDENT_AED)
+		{
+		  grub_error (GRUB_ERR_BAD_FS, "invalid aed tag");
+		  goto fail;
+		}
+
+	      len = U32 (extension->ae_len);
+	      ad = (struct grub_udf_long_ad *)
+		    (buf + sizeof (struct grub_udf_aed));
+	      continue;
+	    }
+	      
+	  if (filebytes < adlen)
+	    {
+	      grub_uint32_t ad_block_num = ad->block.block_num;
+	      grub_uint32_t ad_part_ref = ad->block.part_ref;
+	      grub_free (buf);
+	      return ((U32 (ad_block_num) & GRUB_UDF_EXT_MASK) ?  0 :
+		      (grub_udf_get_block (node->data, ad_part_ref,
+					   ad_block_num)
+		       + (filebytes >> (GRUB_DISK_SECTOR_BITS
+				        + node->data->lbshift))));
+	    }
 
-	  filebytes -= U32 (ad->length);
+	  filebytes -= adlen;
 	  ad++;
-	  len--;
+	  len -= sizeof (struct grub_udf_long_ad);
 	}
     }
 
+fail:
+  if (buf)
+    grub_free (buf);
+
   return 0;
 }
 

[-- Attachment #9: 4-udf-ad-extents-v2.changelog --]
[-- Type: application/octet-stream, Size: 515 bytes --]

	Add support for allocation extent descriptors, needed on fragmented
	volumes.

	* grub-core/fs/udf.c (grub_udf_aed): New struct.
	(grub_udf_read_block): Change type of variable "len" to grub_ssize_t.
	Add sanity check for file entry type.
	Replace divisions with right shifts.
	Check the upper bits of the allocation descriptor's length and honour
	them by loading an allocation extent descriptor if they indicate so.
	Change all failure return paths to free the allocation extent
	descriptor if it was allocated.

  reply	other threads:[~2010-11-11  8:56 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-02 13:10 UDF filesystem fixes for grub Giuseppe Caizzone
2010-11-06 19:59 ` Vladimir 'φ-coder/phcoder' Serbinenko
2010-11-11  8:56   ` Giuseppe Caizzone [this message]
2010-11-14 16:05     ` Vladimir 'φ-coder/phcoder' Serbinenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AANLkTikbwN98s4dw_6zaFLdHA80e44Ri1TxhqFDsT78n@mail.gmail.com \
    --to=acaizzo@gmail.com \
    --cc=grub-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).