Re: latest grub2 & unaligned pointer in gfxmenu

Re: latest grub2 & unaligned pointer in gfxmenu

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 15.02.2015 16:57, Toomas Soome wrote:
> 
> ah, nevermind the gfxmenu;
> 
> the bug was triggered by [ -s filename ] test, with non-existing file. what happens is that grub_zfs_open() will get an error,  calls zfs_unmount and returns error code. then grub_file_open() jumps to fail_ label, calls grub_file_close(), which in turn gets back to zfs_unmount() and you get null pointer dereferences. 
> 
> so, the bug is about zfs_unmount() function and the fix is also simple, zfs_unmount should return if data == NULL.
> 
Sorry, it doesn't make any sense. In kern/file.c at fail label you have:
  if (device)
    grub_device_close (device);

  /* if (net) grub_net_close (net);  */

  grub_free (file);

  grub_memcpy (grub_file_filters_enabled, grub_file_filters_all,
	       sizeof (grub_file_filters_enabled));

Which has only grub_free, no grub_file_close call. Do you see sth
different there?
What kind of ZFS is it? Is it a simple one-device one or something more
complicated?
> 
> 
>> On 12.02.2015, at 20:28, Toomas Soome <tsoome@me.com> wrote:
>>
>>
>> hi!
>>
>> is there any known bugs related gfxmenu?  i’m getting unaligned pointer error on binary built on illumos (gcc-4.4.4).
>>
>> the issue only appears when theme is configured in grub.cfg, without it and gfxterm in cli mode does work.
>>
>> rgds,
>> toomas
> 
> 



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

Re: latest grub2 & unaligned pointer in gfxmenu

[Toomas Soome]

> On 16.02.2015, at 10:14, Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> wrote:
> 
> On 15.02.2015 16:57, Toomas Soome wrote:
>> 
>> ah, nevermind the gfxmenu;
>> 
>> the bug was triggered by [ -s filename ] test, with non-existing file. what happens is that grub_zfs_open() will get an error,  calls zfs_unmount and returns error code. then grub_file_open() jumps to fail_ label, calls grub_file_close(), which in turn gets back to zfs_unmount() and you get null pointer dereferences. 
>> 
>> so, the bug is about zfs_unmount() function and the fix is also simple, zfs_unmount should return if data == NULL.
>> 
> Sorry, it doesn't make any sense. In kern/file.c at fail label you have:
>  if (device)
>    grub_device_close (device);
> 
>  /* if (net) grub_net_close (net);  */
> 
>  grub_free (file);
> 
>  grub_memcpy (grub_file_filters_enabled, grub_file_filters_all,
> 	       sizeof (grub_file_filters_enabled));
> 
> Which has only grub_free, no grub_file_close call. Do you see sth
> different there?
> What kind of ZFS is it? Is it a simple one-device one or something more
> complicated?


ah, damn, i forgot to check against master branch, yes indeed, kern/file.c is different, apparently I have messed it up somehow:D with current version from master it makes more sense;) time to double check the git diff master;)

the zfs layout itself does not matter, the defect appeared on both single disk and raidz, as the root cause was the an attempt to dereference NULL pointer as an result of second call to zfs_unmount. but as we already established, somehow the grub_file_open() code itself was missing update and current code should help to prevent it (altho I would still have NULL pointer check in zfs_unmount() ;)

so, sorry for noise:)

rgds,
toomas