grub2 claims grub.cfg is out of partition

grub2 claims grub.cfg is out of partition

[Svante Signell]

Have you seen this: Debian bug #634799. Seems to be a problem with Linux
on kvm too.

Re: grub2 claims grub.cfg is out of partition

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 20.07.2011 23:50, Svante Signell wrote:
> Have you seen this: Debian bug #634799. Seems to be a problem with Linux
> on kvm too.
>
Such bug forwarding requiring the potential bugfixer to search the
actual description somewhere lese is automatically ignored
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

Re: grub2 claims grub.cfg is out of partition

[Svante Signell]

On Thu, 2011-07-21 at 00:02 +0200, Vladimir 'φ-coder/phcoder' Serbinenko
wrote:
> On 20.07.2011 23:50, Svante Signell wrote:
> > Have you seen this: Debian bug #634799. Seems to be a problem with Linux
> > on kvm too.
> >
> Such bug forwarding requiring the potential bugfixer to search the
> actual description somewhere lese is automatically ignored

Sorry: Here is the link.
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=634799

Re: grub2 claims grub.cfg is out of partition

[Samuel Thibault]

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

tags 634799 + upstream patch
thanks

Hello,

Digging a bit with the issue, it seems grub2 does not ignore entries
with inode == 0 while it should. See linux' ext2_readdir() in
fs/ext2/dir.c doing it:

		for ( ;(char*)de <= limit; de = ext2_next_entry(de)) {
			if (de->inode) {
			  ...
			}
		}

The attached patch fixes it. Shall I commit it to upstream grub2?

Samuel

[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 421 bytes --]

=== modified file 'grub-core/fs/ext2.c'
--- grub-core/fs/ext2.c	2011-04-11 21:01:51 +0000
+++ grub-core/fs/ext2.c	2011-07-20 22:20:43 +0000
@@ -689,7 +689,7 @@ grub_ext2_iterate_dir (grub_fshelp_node_
       if (dirent.direntlen == 0)
         return 0;
 
-      if (dirent.namelen != 0)
+      if (dirent.inode != 0 && dirent.namelen != 0)
 	{
 	  char filename[dirent.namelen + 1];
 	  struct grub_fshelp_node *fdiro;

Re: grub2 claims grub.cfg is out of partition

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 21.07.2011 00:28, Samuel Thibault wrote:
> tags 634799 + upstream patch
> thanks
>
> Hello,
>
I've looked into it and arrived to the similar conclusion that the
grub.cfg.new with inode = 0 is the problem. However I think no such
entries should be there in the first place. In any case GRUB must be
robust to such corruptions but I'd recommend to fix Hurd ext2
implementation as well.
> Digging a bit with the issue, it seems grub2 does not ignore entries
> with inode == 0 while it should. See linux' ext2_readdir() in
> fs/ext2/dir.c doing it:
>
> 		for ( ;(char*)de <= limit; de = ext2_next_entry(de)) {
> 			if (de->inode) {
> 			  ...
> 			}
> 		}
>
> The attached patch fixes it. Shall I commit it to upstream grub2?
>
> Samuel


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

Re: grub2 claims grub.cfg is out of partition

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 21.07.2011 00:28, Samuel Thibault wrote:
> The attached patch fixes it. Shall I commit it to upstream grub2?
>
Go ahead
> Samuel
> patch
>
>
> === modified file 'grub-core/fs/ext2.c'
> --- grub-core/fs/ext2.c	2011-04-11 21:01:51 +0000
> +++ grub-core/fs/ext2.c	2011-07-20 22:20:43 +0000
> @@ -689,7 +689,7 @@ grub_ext2_iterate_dir (grub_fshelp_node_
>        if (dirent.direntlen == 0)
>          return 0;
>  
> -      if (dirent.namelen != 0)
> +      if (dirent.inode != 0 && dirent.namelen != 0)
>  	{
>  	  char filename[dirent.namelen + 1];
>  	  struct grub_fshelp_node *fdiro;
>


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

Re: grub2 claims grub.cfg is out of partition

[Samuel Thibault]

Vladimir 'φ-coder/phcoder' Serbinenko, le Thu 21 Jul 2011 00:31:51 +0200, a écrit :
> On 21.07.2011 00:28, Samuel Thibault wrote:
> > tags 634799 + upstream patch
> > thanks
> >
> > Hello,
> >
> I've looked into it and arrived to the similar conclusion that the
> grub.cfg.new with inode = 0 is the problem. However I think no such
> entries should be there in the first place.

Mmm, the following spec seems to say it is valid:

http://www.nongnu.org/ext2-doc/ext2.html#IFDIR-INODE

A value of 0 indicate that the entry is not used.

Samuel

Re: grub2 claims grub.cfg is out of partition

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 21.07.2011 00:54, Samuel Thibault wrote:
> Vladimir 'φ-coder/phcoder' Serbinenko, le Thu 21 Jul 2011 00:31:51 +0200, a écrit :
>> On 21.07.2011 00:28, Samuel Thibault wrote:
>>> tags 634799 + upstream patch
>>> thanks
>>>
>>> Hello,
>>>
>> I've looked into it and arrived to the similar conclusion that the
>> grub.cfg.new with inode = 0 is the problem. However I think no such
>> entries should be there in the first place.
> Mmm, the following spec seems to say it is valid:
>
> http://www.nongnu.org/ext2-doc/ext2.html#IFDIR-INODE
>
> A value of 0 indicate that the entry is not used.
>
Even in this case I'd recommend avoiding creating such entries since
various third-party extN software (inclusive readers for other OS) is
tailored for Linux flavour. Being nearer to this de-facto reference
would facilitate interoperation. (but as I said GRUB implementation is
to be fixed regardless)
> Samuel
>


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]