Re: [PATCH] fs/xfs: Handle non-continuous data blocks in directory extents
Is the pointer guaranteed to be aligned? If not you have to use unaligned access. Any reason not to check it against correct magic and not just zero-out? Le dim. 11 févr. 2024, 18:36, Jon DeVree a écrit : > The directory extent list does not have to be a continuous list of data > blocks. When GRUB tries to read a non-existant member of the list, > grub_xfs_read_file() will return a block of zero'ed memory. Checking for > a zero'ed magic number is sufficient to skip this non-existant data > block. > > Prior to commit 07318ee7e this was handled as a subtle side effect of > reading the (non-existant) tail data structure. Since the block was > zero'ed the computation of the number of directory entries in the block > would return 0 as well. > > Fixes: 07318ee7e (fs/xfs: Fix XFS directory extent parsing) > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2254370 > Signed-off-by: Jon DeVree > --- > grub-core/fs/xfs.c | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c > index bc2224dbb..a6cce9cfe 100644 > --- a/grub-core/fs/xfs.c > +++ b/grub-core/fs/xfs.c > @@ -902,6 +902,7 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir, > grub_xfs_first_de(dir->data, > dirblock); > int entries = -1; > char *end = dirblock + dirblk_size; > + grub_uint32_t magic; > > numread = grub_xfs_read_file (dir, 0, 0, > blk << dirblk_log2, > @@ -912,6 +913,15 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir, > return 0; > } > > + /* > +* If this data block isn't actually part of the extent list > then > +* grub_xfs_read_file() returns a block of zeros. So if the > magic number > +* field is all zeros then this block should be skipped. > +*/ > + magic = *(grub_uint32_t *)dirblock; > + if (!magic) > + continue; > + > /* > * Leaf and tail information are only in the data block if the > number > * of extents is 1. > -- > 2.43.0 > > > ___ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel > ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] fs/xfs: Handle non-continuous data blocks in directory extents
Thank you, Jon. I am also happy to test. Nicolas and I are all too familiar with this now! -marta On 2/15/24 14:51, Daniel Kiper wrote: Adding Marta, Nicolas, Sebastian and Vladimir... Jon, thank you for the patch! Marta, Nicolas and Sebastian, may I ask you to run tests with this fix? Daniel On Sun, Feb 11, 2024 at 10:34:58AM -0500, Jon DeVree wrote: The directory extent list does not have to be a continuous list of data blocks. When GRUB tries to read a non-existant member of the list, grub_xfs_read_file() will return a block of zero'ed memory. Checking for a zero'ed magic number is sufficient to skip this non-existant data block. Prior to commit 07318ee7e this was handled as a subtle side effect of reading the (non-existant) tail data structure. Since the block was zero'ed the computation of the number of directory entries in the block would return 0 as well. Fixes: 07318ee7e (fs/xfs: Fix XFS directory extent parsing) Fixes:https://bugzilla.redhat.com/show_bug.cgi?id=2254370 Signed-off-by: Jon DeVree --- grub-core/fs/xfs.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c index bc2224dbb..a6cce9cfe 100644 --- a/grub-core/fs/xfs.c +++ b/grub-core/fs/xfs.c @@ -902,6 +902,7 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir, grub_xfs_first_de(dir->data, dirblock); int entries = -1; char *end = dirblock + dirblk_size; + grub_uint32_t magic; numread = grub_xfs_read_file (dir, 0, 0, blk << dirblk_log2, @@ -912,6 +913,15 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir, return 0; } + /* +* If this data block isn't actually part of the extent list then +* grub_xfs_read_file() returns a block of zeros. So if the magic number +* field is all zeros then this block should be skipped. +*/ + magic = *(grub_uint32_t *)dirblock; + if (!magic) + continue; + /* * Leaf and tail information are only in the data block if the number * of extents is 1. -- 2.43.0___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] fs/xfs: Handle non-continuous data blocks in directory extents
On Thu, Feb 15, 2024 at 2:51 PM Daniel Kiper wrote: > > Adding Marta, Nicolas, Sebastian and Vladimir... > > Jon, thank you for the patch! > > Marta, Nicolas and Sebastian, may I ask you to run tests with this fix? Thanks Jon, Daniel. Sure, I will give it a try. I was actually about to send my own version for review. > > Daniel > > On Sun, Feb 11, 2024 at 10:34:58AM -0500, Jon DeVree wrote: > > The directory extent list does not have to be a continuous list of data > > blocks. When GRUB tries to read a non-existant member of the list, > > grub_xfs_read_file() will return a block of zero'ed memory. Checking for > > a zero'ed magic number is sufficient to skip this non-existant data > > block. > > > > Prior to commit 07318ee7e this was handled as a subtle side effect of > > reading the (non-existant) tail data structure. Since the block was > > zero'ed the computation of the number of directory entries in the block > > would return 0 as well. > > > > Fixes: 07318ee7e (fs/xfs: Fix XFS directory extent parsing) > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2254370 > > Signed-off-by: Jon DeVree > > --- > > grub-core/fs/xfs.c | 10 ++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c > > index bc2224dbb..a6cce9cfe 100644 > > --- a/grub-core/fs/xfs.c > > +++ b/grub-core/fs/xfs.c > > @@ -902,6 +902,7 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir, > > grub_xfs_first_de(dir->data, > > dirblock); > > int entries = -1; > > char *end = dirblock + dirblk_size; > > + grub_uint32_t magic; > > > > numread = grub_xfs_read_file (dir, 0, 0, > > blk << dirblk_log2, > > @@ -912,6 +913,15 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir, > > return 0; > > } > > > > + /* > > + * If this data block isn't actually part of the extent list then > > + * grub_xfs_read_file() returns a block of zeros. So if the magic > > number > > + * field is all zeros then this block should be skipped. > > + */ > > + magic = *(grub_uint32_t *)dirblock; > > + if (!magic) > > + continue; > > + > > /* > >* Leaf and tail information are only in the data block if the > > number > >* of extents is 1. > > -- > > 2.43.0 > ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] fs/xfs: Handle non-continuous data blocks in directory extents
Adding Marta, Nicolas, Sebastian and Vladimir... Jon, thank you for the patch! Marta, Nicolas and Sebastian, may I ask you to run tests with this fix? Daniel On Sun, Feb 11, 2024 at 10:34:58AM -0500, Jon DeVree wrote: > The directory extent list does not have to be a continuous list of data > blocks. When GRUB tries to read a non-existant member of the list, > grub_xfs_read_file() will return a block of zero'ed memory. Checking for > a zero'ed magic number is sufficient to skip this non-existant data > block. > > Prior to commit 07318ee7e this was handled as a subtle side effect of > reading the (non-existant) tail data structure. Since the block was > zero'ed the computation of the number of directory entries in the block > would return 0 as well. > > Fixes: 07318ee7e (fs/xfs: Fix XFS directory extent parsing) > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2254370 > Signed-off-by: Jon DeVree > --- > grub-core/fs/xfs.c | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c > index bc2224dbb..a6cce9cfe 100644 > --- a/grub-core/fs/xfs.c > +++ b/grub-core/fs/xfs.c > @@ -902,6 +902,7 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir, > grub_xfs_first_de(dir->data, dirblock); > int entries = -1; > char *end = dirblock + dirblk_size; > + grub_uint32_t magic; > > numread = grub_xfs_read_file (dir, 0, 0, > blk << dirblk_log2, > @@ -912,6 +913,15 @@ grub_xfs_iterate_dir (grub_fshelp_node_t dir, > return 0; > } > > + /* > + * If this data block isn't actually part of the extent list then > + * grub_xfs_read_file() returns a block of zeros. So if the magic > number > + * field is all zeros then this block should be skipped. > + */ > + magic = *(grub_uint32_t *)dirblock; > + if (!magic) > + continue; > + > /* >* Leaf and tail information are only in the data block if the > number >* of extents is 1. > -- > 2.43.0 ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
