bugGNU GRUB - Bugs: bug #46691, grub-install always fail when...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #46691: grub-install always fail when installing into unpartitioned filesystem using blocklist

Submitted by:  Taketo Kabe <kabe>
Submitted on:  Wed 16 Dec 2015 02:53:02 AM UTC  
 
Category:  Disk & Partition Severity:  Major
Priority:  5 - Normal Item Group:  Software Error
Status:  Fixed Privacy:  Public
Assigned to:  None Originator Name:  kabe
Open/Closed:  Closed Release: 
Release:  Git master Reproducibility:  Every Time
Planned Release:  None

( Jump to the original submission)

Sun 10 Jan 2016 07:44:00 AM UTC, comment #9: 

Pushed as 269a522c7db1f383ccf0998a7cf9c4b1797ea0d9.

Thanks!

Andrei Borzenkov <arvidjaar>
Project Member
Sun 10 Jan 2016 02:09:09 AM UTC, comment #8: 

Andrey's file #35932 patch (is_disk.patch) worked!
Confirmed both on image file and real device.
If this approace fixes all platforms, it's lot better than
by crude alignment patch.

Taketo Kabe <kabe>
Wed 06 Jan 2016 09:00:43 AM UTC, comment #7: 

Actually we already had everything to force page cache coherency on Linux, except GRUB did not detect disks as disks. Attached patch fixes problem I observed.

@Taketo: could you test attached patch?

(file #35932)

Andrei Borzenkov <arvidjaar>
Project Member
Tue 05 Jan 2016 07:57:05 PM UTC, comment #6: 

Please don't align buffer manually. We already have a function which does it and it's cheaper. I'm not thrilled with this approach. It feels more like a workaround. We already tried O_SYNC (or was it O_DIRECT?) and it opens another can of worms, first of which is huge slowness on install. Is there any way to flush process buffers?

Vladimir Serbinenko <phcoder>
Project Administrator
Tue 05 Jan 2016 07:23:40 PM UTC, comment #5: 

I actually hit similar issue in scanning for Linux MD (GRUB reads stale superblock under some conditions), so I guess it needs to go in. Unfortunately your patch does not even compile on 64 bit system. Please clean up and submit to grub-devel for review and discussion.

Remove ChangeLog (we do not use it anymore), use disk->name in debug print (cast to int is wrong), fold long lines, follow braces style, check arguments in grub_dprintf on 64 bit system.

Andrei Borzenkov <arvidjaar>
Project Member
Fri 01 Jan 2016 02:57:18 AM UTC, comment #4: 

> GRUB also performs fsync() which should synchronize filesystem and
> block device state. What you describe sounds more like broken filsystem
> which fails to properly implement fsync(). Now, that was known and I
> won't mind adding workaround for this but I want to understand what we
> are trying to fix.


I still haven't came up to any kernel bug reports.

But as a theory, not only on Linux, when different process opens
a (device) file, one with O_DIRECT (I think Linux do this when mounting),
and other without O_DIRECT (grub), grub could grab stale data.

fsync() gurantees that it's written onto the (device) file,
which I confirmed during debugging, but kernel didn't
flush buffers held by process opened without O_DIRECT.
And this could be a feature of that operating system, not a bug.

So, IMHO if O_DIRECT is available, it's a lot safer and portable.

Taketo Kabe <kabe>
Sat 26 Dec 2015 07:54:05 AM UTC, comment #3: 

> anyone opening file without O_DIRECT could grab stale data
> when someone else writes beneath.


That's very strong statement. It may be true, but I would like to see reference to either kernel discussion or kernel source code.

> grub-install purges its disk cache before core.img verification,
> but this isn't effective since stale data is returned by kernel
> itself.


GRUB also performs fsync() which should synchronize filesystem and block device state. What you describe sounds more like broken filsystem which fails to properly implement fsync(). Now, that was known and I won't mind adding workaround for this but I want to understand what we are trying to fix.

Andrei Borzenkov <arvidjaar>
Project Member
Fri 18 Dec 2015 01:43:26 AM UTC, comment #2: 

This is the 32-bit version of CentOS 7 released at
https://wiki.centos.org/SpecialInterestGroup/AltArch/i386

::::::::::::::
/etc/centos-release
::::::::::::::
CentOS Linux release 7.1.1503 (Final)

uname -a
Linux corvus.five.ten 3.10.0-229.el7.i686 #1 SMP Sun May 17 14:06:40 UTC 2015 i686 i686 i386 GNU/Linux

I couldn't find any kernel bug reports yet, but
I didn't think this is a kernel bug;
anyone opening file without O_DIRECT could grab stale data
when someone else writes beneath.
GRUB Legacy did have some similar fix regarding O_DIRECT.

Taketo Kabe <kabe>
Wed 16 Dec 2015 04:33:51 AM UTC, comment #1: 

Which linux kernel version? Was it reported to kernel (as it sounds like kernel bug)? Do you have pointers to discussion/bug for kernel?

I cannot reproduce it and I do not remember ever seeing it.

Andrei Borzenkov <arvidjaar>
Project Member
Wed 16 Dec 2015 02:53:02 AM UTC, original submission:  

Suppose you wanted to make a FAT-based GRUB2 floppy disk image, as follows:

>>>>

mkdosfs -F 12 -R 2 -I -C -S 512 /tmp/14.img 1440
mount -t vfat -o loop,sync,dirsync,umask=0,shortname=mixed /tmp/14.img /media/fd || exit 1

grub-install \
--boot-directory=/media/fd/boot \
--modules='fat iso9660 biosdisk part_msdos part_bsd part_gpt halt reboot' \
--install-modules='cat linux16 halt reboot help normal ls chain fat ext2 ufs2 ntfs zfs xfs msdospart ' \
--fonts= --locales= --allow-floppy \
--force \
--debug --debug \
/dev/loop0

umount /media/fd
<<<<

grub-install will always fail with

>>>>

grub2-install: warning: Embedding is not possible.  GRUB can only be installed in this setup by using blocklists.  However, blocklists are UNRELIABLE and their use is discouraged..
grub2-install: info: will leave the core image on the filesystem.
grub2-install: info: opening the core image `/media/fd/boot/grub2/i386-pc/core.img'.
grub2-install: info: FIEMAP failed. Reverting to FIBMAP.
grub2-install: info: saving <924,0,512>.
....
grub2-install: info: saving <1018,0,415>.
grub2-install: info: opening the core image `/media/fd/boot/grub2/i386-pc/core.img'.
grub-core/osdep/hostdisk.c:395: reusing open device `/dev/loop0'
grub2-install: error: blocklists are invalid.
<<<<

This is due to bug/feature of accessing unpartitioned device on linux;
suppose block Y of /dev/loop0 is affected.
(1) grub read-aheads block Y from /dev/loop0
(2) grub writes via filesystem the core.img, which correctly rewrites underlying block Y
(3) grub re-reads block Y via /dev/loop0 as blocklist of core.img to
    verify content,
    which kernel returns stale data of (1),
    resulting in verify mismatch; "error: blocklists are invalid"

grub-install purges its disk cache before core.img verification, but this
isn't effective since stale data is returned by kernel itself.

This problem also occurs on real block device /dev/floppy .
This does NOT occur on partitioned device such as /dev/sdb1, for unknown reason;
so did not affect standard HDD installs.

Fix: Open(2) /dev/loop0 with O_DIRECT flag. This fixed the problem.
Patch attached.  Do
% find . -name '*setup_bios.o' |xargs rm
% find . -name '*hostdisk.o' |xargs rm
(or make clean) after applying patch for correct rebuild.

Unfortunately O_DIRECT requires block-aligned buffer for read(2)ing,
so grub_util_biosdisk_read() needs a whole mess of preparing aligned buffer.

----- 8< -----
Open(2) destination device with O_DIRECT flag. This fixes grub-install to fail
when installing using a blocklist to a filesystem mounted via /dev/loop0 loopback.

Taketo Kabe <kabe>

 

Attached Files
file #35932:  is_disk.patch added by arvidjaar (1013B - text/x-patch)
file #35759:  p added by kabe (8KiB - application/octet-stream - O_DIRECT patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by phcoder (Posted a comment)
  • -email is unavailable- added by arvidjaar (Posted a comment)
  • -email is unavailable- added by kabe (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-01-10 arvidjaar Open/ClosedOpen => Closed
    2016-01-10 arvidjaar StatusNone => Fixed
    2016-01-06 arvidjaar Attached File- => Added is_disk.patch, #35932
    2015-12-16 kabe Attached File- => Added p, #35759

    Back to the top


    Powered by Savane 3.6