Re: [PATCH] Refuse to install on XFS destroying its superblock
Ok, let's try this: We refuse to install on a partition UNLESS: - A filesystem can be identified in it. - This filesystem is known to reserve the first block for DOS-style chainload. If these conditions aren't met, user will have to override our check. Patch attached. Also in people/robertmh/grub-setup-fs-probe. -- Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all." === modified file 'ChangeLog' --- ChangeLog 2009-10-24 23:13:27 + +++ ChangeLog 2009-10-24 23:57:58 + @@ -1,5 +1,15 @@ 2009-10-25 Robert Millan + * include/grub/fs.h (struct grub_fs): Add `reserved_first_sector' + member. + * fs/ext2.c (grub_ext2_fs): Initialize `reserved_first_sector' to 1. + * util/i386/pc/grub-setup.c (setup): Add safety check that probes for + filesystems which begin at first sector. + (options): New option --skip-fs-probe. + (main): Handle --skip-fs-probe and pass it to setup(). + +2009-10-25 Robert Millan + * fs/cpio.c [MODE_USTAR]: Finish `tar' module instead of `cpio'. [! MODE_USTAR]: Finish `cpio' module instead of `tar'. === modified file 'fs/ext2.c' --- fs/ext2.c 2009-07-19 13:59:21 + +++ fs/ext2.c 2009-10-24 23:57:58 + @@ -924,6 +924,7 @@ .label = grub_ext2_label, .uuid = grub_ext2_uuid, .mtime = grub_ext2_mtime, +.reserved_first_sector = 1, .next = 0 }; === modified file 'include/grub/fs.h' --- include/grub/fs.h 2009-06-10 21:04:23 + +++ include/grub/fs.h 2009-10-24 23:57:58 + @@ -68,6 +68,11 @@ /* Get writing time of filesystem. */ grub_err_t (*mtime) (grub_device_t device, grub_int32_t *timebuf); +#ifdef GRUB_UTIL + /* Whether this filesystem reserves first sector for DOS-style boot. */ + int reserved_first_sector; +#endif + /* The next filesystem. */ struct grub_fs *next; }; === modified file 'util/i386/pc/grub-setup.c' --- util/i386/pc/grub-setup.c 2009-08-25 08:28:13 + +++ util/i386/pc/grub-setup.c 2009-10-24 23:57:58 + @@ -86,7 +86,7 @@ static void setup (const char *dir, const char *boot_file, const char *core_file, - const char *root, const char *dest, int must_embed, int force) + const char *root, const char *dest, int must_embed, int force, int fs_probe) { char *boot_path, *core_path, *core_path_dev; char *boot_img, *core_img; @@ -251,6 +251,21 @@ if (grub_disk_read (dest_dev->disk, 0, 0, GRUB_DISK_SECTOR_SIZE, tmp_img)) grub_util_error ("%s", grub_errmsg); + if (dest_dev->disk->partition && fs_probe) +{ + grub_fs_t fs; + fs = grub_fs_probe (dest_dev); + if (! fs) + grub_util_error ("Unable to identify a filesystem in %s; safety check can't be performed."); + + if (! fs->reserved_first_sector) + grub_util_error ("%s appears to contain a %s filesystem which isn't known to " + "reserve space for DOS-style boot. Installing GRUB there could " + "result in FILESYSTEM DESTRUCTION if valuable data is overwritten " + "by grub-setup (--skip-fs-probe disables this " + "check, use at your own risk).", dest_dev->disk->name, fs->name); +} + /* Copy the possible DOS BPB. */ memcpy (boot_img + GRUB_BOOT_MACHINE_BPB_START, tmp_img + GRUB_BOOT_MACHINE_BPB_START, @@ -556,6 +571,7 @@ {"device-map", required_argument, 0, 'm'}, {"root-device", required_argument, 0, 'r'}, {"force", no_argument, 0, 'f'}, +{"skip-fs-probe", no_argument, 0, 's'}, {"help", no_argument, 0, 'h'}, {"version", no_argument, 0, 'V'}, {"verbose", no_argument, 0, 'v'}, @@ -580,6 +596,7 @@ -m, --device-map=FILE use FILE as the device map [default=%s]\n\ -r, --root-device=DEV use DEV as the root device [default=guessed]\n\ -f, --force install even if problems are detected\n\ + -s, --skip-fs-probe do not probe for filesystems in DEVICE\n\ -h, --help display this message and exit\n\ -V, --version print version information and exit\n\ -v, --verbose print verbose messages\n\ @@ -613,7 +630,7 @@ char *dev_map = 0; char *root_dev = 0; char *dest_dev; - int must_embed = 0, force = 0; + int must_embed = 0, force = 0, fs_probe = 1; progname = "grub-setup"; @@ -666,6 +683,10 @@ force = 1; break; + case 's': + fs_probe = 0; + break; + case 'h': usage (0); break; @@ -767,7 +788,7 @@ setup (dir ? : DEFAULT_DIRECTORY, boot_file ? : DEFAULT_BOOT_FILE, core_file ? : DEFAULT_CORE_FILE, - root_dev, grub_util_get_grub_dev (devicelist[i]), 1, force); + root_dev, grub_util_get_grub_dev (devicelist[i]), 1, force, fs_probe); } } else @@ -776,7 +797,7 @@ setup (dir ? : DEFAULT_DIRECTORY, boot_file ? : DEFAULT_BOOT_FILE, core_file ? : DEFAULT_CORE_FILE, - root_dev, dest_dev, must_embed, force); +
Re: [PATCH] Refuse to install on XFS destroying its superblock
Robert Millan wrote: > On Sun, Oct 18, 2009 at 06:30:11PM +0200, Vladimir 'phcoder' Serbinenko wrote: > >> Robert Millan wrote: >> >>> On Sat, Oct 17, 2009 at 02:09:31PM +0200, Vladimir 'phcoder' Serbinenko >>> wrote: >>> >>> >> The danger is that fs_probe may reject filesystem as valid just because >> it's newer than expected. >> >> >> > What do you mean with "reject filesystem as valid"? > > > > Sorry for being unclear. I just meant that if some XFS structures are updated then our xfs driver won't recognise it as xfs >>> What do you mean with "updated"? You mean a new implementation of XFS? Or >>> the same instance of XFS that has been modified after use? >>> >>> >>> >> I mean next version on XFS >> > > Sorry, but what did you expect? You want to prevent PEBCAK using > heuristic. There's no way we can tell if those 512 bytes are valuable > data, only the user can. And even if you try to err on the safest side, > there's no garantee that newer versions of XFS, or other filesystems that > don't even exist yet will be detectable by us no matter what we do. > > I think best way is to have whitelist of OS which have first sector free and possible manual override like it was suggested. > Why don't we just take a backup like someone suggested a while ago? I > think there was even a patch. This way if valuable data is lost, user can > restore it (and while at it, learnt his lesson that filesystems and embedded > code aren't really supposed to be mixed in the same place). > > The backup is inevitably stored on the filesystem itself which becomes inaccessible once superblock is destroyed. -- Regards Vladimir 'phcoder' Serbinenko Personal git repository: http://repo.or.cz/w/grub2/phcoder.git ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Refuse to install on XFS destroying its superblock
On Sun, Oct 18, 2009 at 06:30:11PM +0200, Vladimir 'phcoder' Serbinenko wrote: > Robert Millan wrote: > > On Sat, Oct 17, 2009 at 02:09:31PM +0200, Vladimir 'phcoder' Serbinenko > > wrote: > > > The danger is that fs_probe may reject filesystem as valid just because > it's newer than expected. > > > >>> What do you mean with "reject filesystem as valid"? > >>> > >>> > >>> > >> Sorry for being unclear. I just meant that if some XFS structures are > >> updated then our xfs driver won't recognise it as xfs > >> > > > > What do you mean with "updated"? You mean a new implementation of XFS? Or > > the same instance of XFS that has been modified after use? > > > > > I mean next version on XFS Sorry, but what did you expect? You want to prevent PEBCAK using heuristic. There's no way we can tell if those 512 bytes are valuable data, only the user can. And even if you try to err on the safest side, there's no garantee that newer versions of XFS, or other filesystems that don't even exist yet will be detectable by us no matter what we do. Why don't we just take a backup like someone suggested a while ago? I think there was even a patch. This way if valuable data is lost, user can restore it (and while at it, learnt his lesson that filesystems and embedded code aren't really supposed to be mixed in the same place). -- Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all." ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Refuse to install on XFS destroying its superblock
richardvo...@gmail.com wrote: > On Sat, Oct 17, 2009 at 7:09 AM, Vladimir 'phcoder' Serbinenko > wrote: > >> Robert Millan wrote: >> >>> On Sat, Oct 17, 2009 at 01:43:37PM +0200, Vladimir 'phcoder' Serbinenko >>> wrote: >>> >>> Robert Millan wrote: > On Sat, Oct 17, 2009 at 12:18:05AM +0200, Vladimir 'phcoder' Serbinenko > wrote: > > > >> 2009-10-16 Vladimir Serbinenko >> >> + * util/i386/pc/grub-setup.c (setup): Refuse to overwrite XFS >> superblock. >> + (options): New option --destroy-xfs. >> + (main): Handle --destroy-xfs. >> >> >> > I gave this some more thought, and I think this could be less ad-hoc. > We're > treating XFS as if it were a "weird", unique thing just because it isn't > biased > towards DOS-style boot like most filesystems are. > > Instead, I've done something more generic, using our standard filesystem > probing engine which should be more reliable than a single memcmp. > > > > The danger is that fs_probe may reject filesystem as valid just because it's newer than expected. >>> What do you mean with "reject filesystem as valid"? >>> >>> >>> >> Sorry for being unclear. I just meant that if some XFS structures are >> updated then our xfs driver won't recognise it as xfs >> > > > Then instead of blacklisting xfs, why not whitelist filesystems which > are known to have usable blocks for embedding (doesn't the number of > blocks vary with filesystem anyway?) and an override parameter > (--into-unrecognized-fs). > > In long term it's the best possibility but in this case we're too near to a release. What I want is just to avoid XFS pitfall many users may step into >> -- >> Regards >> Vladimir 'phcoder' Serbinenko >> Personal git repository: http://repo.or.cz/w/grub2/phcoder.git >> >> >> >> ___ >> Grub-devel mailing list >> Grub-devel@gnu.org >> http://lists.gnu.org/mailman/listinfo/grub-devel >> >> > > > ___ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > > -- Regards Vladimir 'phcoder' Serbinenko Personal git repository: http://repo.or.cz/w/grub2/phcoder.git ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Refuse to install on XFS destroying its superblock
Robert Millan wrote: > On Sat, Oct 17, 2009 at 02:09:31PM +0200, Vladimir 'phcoder' Serbinenko wrote: > The danger is that fs_probe may reject filesystem as valid just because it's newer than expected. >>> What do you mean with "reject filesystem as valid"? >>> >>> >>> >> Sorry for being unclear. I just meant that if some XFS structures are >> updated then our xfs driver won't recognise it as xfs >> > > What do you mean with "updated"? You mean a new implementation of XFS? Or > the same instance of XFS that has been modified after use? > > I mean next version on XFS -- Regards Vladimir 'phcoder' Serbinenko Personal git repository: http://repo.or.cz/w/grub2/phcoder.git ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Refuse to install on XFS destroying its superblock
On Sat, Oct 17, 2009 at 02:09:31PM +0200, Vladimir 'phcoder' Serbinenko wrote: > >> The danger is that fs_probe may reject filesystem as valid just because > >> it's newer than expected. > >> > > > > What do you mean with "reject filesystem as valid"? > > > > > Sorry for being unclear. I just meant that if some XFS structures are > updated then our xfs driver won't recognise it as xfs What do you mean with "updated"? You mean a new implementation of XFS? Or the same instance of XFS that has been modified after use? -- Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all." ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Refuse to install on XFS destroying its superblock
On Sat, Oct 17, 2009 at 7:09 AM, Vladimir 'phcoder' Serbinenko wrote: > Robert Millan wrote: >> On Sat, Oct 17, 2009 at 01:43:37PM +0200, Vladimir 'phcoder' Serbinenko >> wrote: >> >>> Robert Millan wrote: >>> On Sat, Oct 17, 2009 at 12:18:05AM +0200, Vladimir 'phcoder' Serbinenko wrote: > 2009-10-16 Vladimir Serbinenko > > + * util/i386/pc/grub-setup.c (setup): Refuse to overwrite XFS > superblock. > + (options): New option --destroy-xfs. > + (main): Handle --destroy-xfs. > > I gave this some more thought, and I think this could be less ad-hoc. We're treating XFS as if it were a "weird", unique thing just because it isn't biased towards DOS-style boot like most filesystems are. Instead, I've done something more generic, using our standard filesystem probing engine which should be more reliable than a single memcmp. >>> The danger is that fs_probe may reject filesystem as valid just because >>> it's newer than expected. >>> >> >> What do you mean with "reject filesystem as valid"? >> >> > Sorry for being unclear. I just meant that if some XFS structures are > updated then our xfs driver won't recognise it as xfs Then instead of blacklisting xfs, why not whitelist filesystems which are known to have usable blocks for embedding (doesn't the number of blocks vary with filesystem anyway?) and an override parameter (--into-unrecognized-fs). > > > -- > Regards > Vladimir 'phcoder' Serbinenko > Personal git repository: http://repo.or.cz/w/grub2/phcoder.git > > > > ___ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Refuse to install on XFS destroying its superblock
Robert Millan wrote: > On Sat, Oct 17, 2009 at 01:43:37PM +0200, Vladimir 'phcoder' Serbinenko wrote: > >> Robert Millan wrote: >> >>> On Sat, Oct 17, 2009 at 12:18:05AM +0200, Vladimir 'phcoder' Serbinenko >>> wrote: >>> >>> 2009-10-16 Vladimir Serbinenko + * util/i386/pc/grub-setup.c (setup): Refuse to overwrite XFS superblock. + (options): New option --destroy-xfs. + (main): Handle --destroy-xfs. >>> I gave this some more thought, and I think this could be less ad-hoc. We're >>> treating XFS as if it were a "weird", unique thing just because it isn't >>> biased >>> towards DOS-style boot like most filesystems are. >>> >>> Instead, I've done something more generic, using our standard filesystem >>> probing engine which should be more reliable than a single memcmp. >>> >>> >>> >> The danger is that fs_probe may reject filesystem as valid just because >> it's newer than expected. >> > > What do you mean with "reject filesystem as valid"? > > Sorry for being unclear. I just meant that if some XFS structures are updated then our xfs driver won't recognise it as xfs -- Regards Vladimir 'phcoder' Serbinenko Personal git repository: http://repo.or.cz/w/grub2/phcoder.git ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Refuse to install on XFS destroying its superblock
Am Samstag, den 17.10.2009, 14:00 +0200 schrieb Robert Millan: > On Sat, Oct 17, 2009 at 01:43:37PM +0200, Vladimir 'phcoder' Serbinenko wrote: > > Robert Millan wrote: > > > On Sat, Oct 17, 2009 at 12:18:05AM +0200, Vladimir 'phcoder' Serbinenko > > > wrote: > > > > > >> 2009-10-16 Vladimir Serbinenko > > >> > > >> +* util/i386/pc/grub-setup.c (setup): Refuse to overwrite XFS > > >> superblock. > > >> +(options): New option --destroy-xfs. > > >> +(main): Handle --destroy-xfs. > > >> > > > > > > I gave this some more thought, and I think this could be less ad-hoc. > > > We're > > > treating XFS as if it were a "weird", unique thing just because it isn't > > > biased > > > towards DOS-style boot like most filesystems are. > > > > > > Instead, I've done something more generic, using our standard filesystem > > > probing engine which should be more reliable than a single memcmp. > > > > > > > > The danger is that fs_probe may reject filesystem as valid just because > > it's newer than expected. > > What do you mean with "reject filesystem as valid"? For example with the extN filesystems we reject them as valid if they use INCOMPAT flags we don't support. For example external ext3/4 journal devices, which caused a reboot or segfault or something like that, before we commited Javier's patch for it. In that case it doestn't matter because the first sector is still unused but for other filesystems this could maybe be a problem. -- Felix Zielcke Proud Debian Maintainer and GNU GRUB developer ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Refuse to install on XFS destroying its superblock
On Sat, Oct 17, 2009 at 01:43:37PM +0200, Vladimir 'phcoder' Serbinenko wrote: > Robert Millan wrote: > > On Sat, Oct 17, 2009 at 12:18:05AM +0200, Vladimir 'phcoder' Serbinenko > > wrote: > > > >> 2009-10-16 Vladimir Serbinenko > >> > >> + * util/i386/pc/grub-setup.c (setup): Refuse to overwrite XFS superblock. > >> + (options): New option --destroy-xfs. > >> + (main): Handle --destroy-xfs. > >> > > > > I gave this some more thought, and I think this could be less ad-hoc. We're > > treating XFS as if it were a "weird", unique thing just because it isn't > > biased > > towards DOS-style boot like most filesystems are. > > > > Instead, I've done something more generic, using our standard filesystem > > probing engine which should be more reliable than a single memcmp. > > > > > The danger is that fs_probe may reject filesystem as valid just because > it's newer than expected. What do you mean with "reject filesystem as valid"? -- Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all." ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Refuse to install on XFS destroying its superblock
Robert Millan wrote: > On Sat, Oct 17, 2009 at 12:18:05AM +0200, Vladimir 'phcoder' Serbinenko wrote: > >> 2009-10-16 Vladimir Serbinenko >> >> +* util/i386/pc/grub-setup.c (setup): Refuse to overwrite XFS superblock. >> +(options): New option --destroy-xfs. >> +(main): Handle --destroy-xfs. >> > > I gave this some more thought, and I think this could be less ad-hoc. We're > treating XFS as if it were a "weird", unique thing just because it isn't > biased > towards DOS-style boot like most filesystems are. > > Instead, I've done something more generic, using our standard filesystem > probing engine which should be more reliable than a single memcmp. > > The danger is that fs_probe may reject filesystem as valid just because it's newer than expected. > I propose the attached patch. > > > > > ___ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel -- Regards Vladimir 'phcoder' Serbinenko Personal git repository: http://repo.or.cz/w/grub2/phcoder.git ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Refuse to install on XFS destroying its superblock
On Sat, Oct 17, 2009 at 12:18:05AM +0200, Vladimir 'phcoder' Serbinenko wrote: > 2009-10-16 Vladimir Serbinenko > > + * util/i386/pc/grub-setup.c (setup): Refuse to overwrite XFS superblock. > + (options): New option --destroy-xfs. > + (main): Handle --destroy-xfs. I gave this some more thought, and I think this could be less ad-hoc. We're treating XFS as if it were a "weird", unique thing just because it isn't biased towards DOS-style boot like most filesystems are. Instead, I've done something more generic, using our standard filesystem probing engine which should be more reliable than a single memcmp. I propose the attached patch. -- Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all." === modified file 'ChangeLog' --- ChangeLog 2009-10-16 20:21:12 + +++ ChangeLog 2009-10-17 11:19:35 + @@ -1,3 +1,13 @@ +2009-10-17 Robert Millan + + * include/grub/fs.h (struct grub_fs): Add `begins_at_first_sector' + member. + * fs/xfs.c (grub_xfs_fs): Initialize `begins_at_first_sector' to 1. + * util/i386/pc/grub-setup.c (setup): Add safety check that probes for + filesystems which begin at first sector. + (options): New option --skip-fs-probe. + (main): Handle --skip-fs-probe and pass it to setup(). + 2009-10-16 Vladimir Serbinenko Let user specify OpenBSD root device. === modified file 'fs/xfs.c' --- fs/xfs.c 2009-08-26 14:17:34 + +++ fs/xfs.c 2009-10-17 11:19:35 + @@ -805,6 +805,9 @@ .close = grub_xfs_close, .label = grub_xfs_label, .uuid = grub_xfs_uuid, +#ifdef GRUB_UTIL +.begins_at_first_sector = 1, +#endif .next = 0 }; === modified file 'include/grub/fs.h' --- include/grub/fs.h 2009-06-10 21:04:23 + +++ include/grub/fs.h 2009-10-17 11:19:35 + @@ -68,6 +68,12 @@ /* Get writing time of filesystem. */ grub_err_t (*mtime) (grub_device_t device, grub_int32_t *timebuf); +#ifdef GRUB_UTIL + /* Whether this filesystem begins at first sector or reserves it for + DOS-style boot. */ + int begins_at_first_sector; +#endif + /* The next filesystem. */ struct grub_fs *next; }; === modified file 'util/i386/pc/grub-setup.c' --- util/i386/pc/grub-setup.c 2009-08-25 08:28:13 + +++ util/i386/pc/grub-setup.c 2009-10-17 11:19:35 + @@ -86,7 +86,7 @@ static void setup (const char *dir, const char *boot_file, const char *core_file, - const char *root, const char *dest, int must_embed, int force) + const char *root, const char *dest, int must_embed, int force, int fs_probe) { char *boot_path, *core_path, *core_path_dev; char *boot_img, *core_img; @@ -251,6 +251,16 @@ if (grub_disk_read (dest_dev->disk, 0, 0, GRUB_DISK_SECTOR_SIZE, tmp_img)) grub_util_error ("%s", grub_errmsg); + if (fs_probe) +{ + grub_fs_t fs; + fs = grub_fs_probe (dest_dev); + if (fs && fs->begins_at_first_sector) + grub_util_error ("%s appears to contain a %s filesystem, which would be " + "overwritten by grub-setup (--skip-fs-probe disables this " + "check, use at your own risk).", dest_dev->disk->name, fs->name); +} + /* Copy the possible DOS BPB. */ memcpy (boot_img + GRUB_BOOT_MACHINE_BPB_START, tmp_img + GRUB_BOOT_MACHINE_BPB_START, @@ -556,6 +566,7 @@ {"device-map", required_argument, 0, 'm'}, {"root-device", required_argument, 0, 'r'}, {"force", no_argument, 0, 'f'}, +{"skip-fs-probe", no_argument, 0, 's'}, {"help", no_argument, 0, 'h'}, {"version", no_argument, 0, 'V'}, {"verbose", no_argument, 0, 'v'}, @@ -580,6 +591,7 @@ -m, --device-map=FILE use FILE as the device map [default=%s]\n\ -r, --root-device=DEV use DEV as the root device [default=guessed]\n\ -f, --force install even if problems are detected\n\ + -s, --skip-fs-probe do not probe for filesystems in DEVICE\n\ -h, --help display this message and exit\n\ -V, --version print version information and exit\n\ -v, --verbose print verbose messages\n\ @@ -613,7 +625,7 @@ char *dev_map = 0; char *root_dev = 0; char *dest_dev; - int must_embed = 0, force = 0; + int must_embed = 0, force = 0, fs_probe = 1; progname = "grub-setup"; @@ -666,6 +678,10 @@ force = 1; break; + case 's': + fs_probe = 0; + break; + case 'h': usage (0); break; @@ -767,7 +783,7 @@ setup (dir ? : DEFAULT_DIRECTORY, boot_file ? : DEFAULT_BOOT_FILE, core_file ? : DEFAULT_CORE_FILE, - root_dev, grub_util_get_grub_dev (devicelist[i]), 1, force); + root_dev, grub_util_get_grub_dev (devicelist[i]), 1, force, fs_probe); } } else @@ -776,7 +792,7 @@ setup (dir ? : DEFAULT_DIRECTORY, boot_file ? : DEFAULT_BOOT_FILE, core_file ? : DEFAULT_CORE_FILE, - root_dev, dest_dev,
Re: [PATCH] Refuse to install on XFS destroying its superblock
Robert Millan wrote: > On Fri, Oct 16, 2009 at 11:08:31PM +0200, Vladimir 'phcoder' Serbinenko wrote: > >> Robert Millan wrote: >> >>> On Fri, Oct 16, 2009 at 10:09:41PM +0200, Vladimir 'phcoder' Serbinenko >>> wrote: >>> >>> Robert Millan wrote: > On Fri, Oct 16, 2009 at 06:01:56PM +0200, Jordi Mallach wrote: > > > >> On Fri, Oct 16, 2009 at 04:03:01PM +0200, Vladimir 'phcoder' Serbinenko >> wrote: >> >> >> >>> + if (memcmp (tmp_img, "XFSB", 4) == 0) >>> +grub_util_error ("Can't install on XFS."); >>> >>> >>> >> Can this error message give some more detail on what the problem is? >> >> >> > I suggest something like: > > grub_util_warn ("Refusing to overwrite XFS meta-data."); > > This is more informative, and with grub_util_warn() user has an > opportunity to > override it if she knows what she's doing. > > > > Installing with blocklists/to partition is considered backward-compatibility feature. We never supported a config with XFS why we would want bw-compat for it? >>> Because we can't reliably tell if it's a config with XFS, only the user can. >>> This is an issue for both MBR or PBR installs. >>> >>> Maybe "XFSB" is only a remnant from one of this disk / partition former >>> lifes. Maybe it's a valid XFS but user no longer cares about it. Or >>> maybe a DOS-style label was created on top of it, without overwriting the >>> first >>> 440 bytes. Or maybe another filesystem had overwritten most XFS metadata >>> but preserved the first block (this is conceivable since other filesystems >>> tend to avoid using the first block). >>> >>> If user has to workaround GRUB heuristics by dd'ing zeros into a partition >>> before running grub-install, this is a sign GRUB isn't doing the right >>> thing. >>> >>> >>> >> Well, ok. But then I would ask to use a separate --force e.g. >> --force-destroy-xfs since users and distributions tend to use --force >> too much >> > > Ok. > > -- Regards Vladimir 'phcoder' Serbinenko Personal git repository: http://repo.or.cz/w/grub2/phcoder.git diff --git a/ChangeLog b/ChangeLog index 960fc06..cc225a7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ 2009-10-16 Vladimir Serbinenko + * util/i386/pc/grub-setup.c (setup): Refuse to overwrite XFS superblock. + (options): New option --destroy-xfs. + (main): Handle --destroy-xfs. + +2009-10-16 Vladimir Serbinenko + Let user specify OpenBSD root device. * loader/i386/bsd.c (openbsd_root): New variable. diff --git a/util/i386/pc/grub-setup.c b/util/i386/pc/grub-setup.c index ccfbd1d..4ef746b 100644 --- a/util/i386/pc/grub-setup.c +++ b/util/i386/pc/grub-setup.c @@ -86,7 +86,8 @@ grub_refresh (void) static void setup (const char *dir, const char *boot_file, const char *core_file, - const char *root, const char *dest, int must_embed, int force) + const char *root, const char *dest, int must_embed, int force, + int destroy_xfs) { char *boot_path, *core_path, *core_path_dev; char *boot_img, *core_img; @@ -251,6 +252,13 @@ setup (const char *dir, if (grub_disk_read (dest_dev->disk, 0, 0, GRUB_DISK_SECTOR_SIZE, tmp_img)) grub_util_error ("%s", grub_errmsg); + if (memcmp (tmp_img, "XFSB", 4) == 0) +{ + grub_util_warn ("This install would require destroying XFS."); + if (! destroy_xfs) + grub_util_error ("If you really want to destroy it use --destroy-xfs."); +} + /* Copy the possible DOS BPB. */ memcpy (boot_img + GRUB_BOOT_MACHINE_BPB_START, tmp_img + GRUB_BOOT_MACHINE_BPB_START, @@ -556,6 +564,7 @@ static struct option options[] = {"device-map", required_argument, 0, 'm'}, {"root-device", required_argument, 0, 'r'}, {"force", no_argument, 0, 'f'}, +{"destroy-xfs", no_argument, 0, 'X'}, {"help", no_argument, 0, 'h'}, {"version", no_argument, 0, 'V'}, {"verbose", no_argument, 0, 'v'}, @@ -580,6 +589,7 @@ DEVICE must be a GRUB device (e.g. ``(hd0,1)'').\n\ -m, --device-map=FILE use FILE as the device map [default=%s]\n\ -r, --root-device=DEV use DEV as the root device [default=guessed]\n\ -f, --force install even if problems are detected\n\ + --destroy-xfs install even destroying XFS superblock\n\ -h, --help display this message and exit\n\ -V, --version print version information and exit\n\ -v, --verbose print verbose messages\n\ @@ -614,6 +624,7 @@ main (int argc, char *argv[]) char *root_dev = 0; char *dest_dev; int must_embed = 0, force = 0; + int destroy_xfs = 0; progname = "grub-setup"; @@ -666,6 +677,10 @@ main (int argc, char *argv[]) force = 1;
Re: [PATCH] Refuse to install on XFS destroying its superblock
On Fri, Oct 16, 2009 at 11:08:31PM +0200, Vladimir 'phcoder' Serbinenko wrote: > Robert Millan wrote: > > On Fri, Oct 16, 2009 at 10:09:41PM +0200, Vladimir 'phcoder' Serbinenko > > wrote: > > > >> Robert Millan wrote: > >> > >>> On Fri, Oct 16, 2009 at 06:01:56PM +0200, Jordi Mallach wrote: > >>> > >>> > On Fri, Oct 16, 2009 at 04:03:01PM +0200, Vladimir 'phcoder' Serbinenko > wrote: > > > > + if (memcmp (tmp_img, "XFSB", 4) == 0) > > +grub_util_error ("Can't install on XFS."); > > > > > Can this error message give some more detail on what the problem is? > > > >>> I suggest something like: > >>> > >>> grub_util_warn ("Refusing to overwrite XFS meta-data."); > >>> > >>> This is more informative, and with grub_util_warn() user has an > >>> opportunity to > >>> override it if she knows what she's doing. > >>> > >>> > >>> > >> Installing with blocklists/to partition is considered > >> backward-compatibility feature. We never supported a config with XFS why > >> we would want bw-compat for it? > >> > > > > Because we can't reliably tell if it's a config with XFS, only the user can. > > This is an issue for both MBR or PBR installs. > > > > Maybe "XFSB" is only a remnant from one of this disk / partition former > > lifes. Maybe it's a valid XFS but user no longer cares about it. Or > > maybe a DOS-style label was created on top of it, without overwriting the > > first > > 440 bytes. Or maybe another filesystem had overwritten most XFS metadata > > but preserved the first block (this is conceivable since other filesystems > > tend to avoid using the first block). > > > > If user has to workaround GRUB heuristics by dd'ing zeros into a partition > > before running grub-install, this is a sign GRUB isn't doing the right > > thing. > > > > > Well, ok. But then I would ask to use a separate --force e.g. > --force-destroy-xfs since users and distributions tend to use --force > too much Ok. -- Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all." ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Refuse to install on XFS destroying its superblock
Robert Millan wrote: > On Fri, Oct 16, 2009 at 10:09:41PM +0200, Vladimir 'phcoder' Serbinenko wrote: > >> Robert Millan wrote: >> >>> On Fri, Oct 16, 2009 at 06:01:56PM +0200, Jordi Mallach wrote: >>> >>> On Fri, Oct 16, 2009 at 04:03:01PM +0200, Vladimir 'phcoder' Serbinenko wrote: > + if (memcmp (tmp_img, "XFSB", 4) == 0) > +grub_util_error ("Can't install on XFS."); > > Can this error message give some more detail on what the problem is? >>> I suggest something like: >>> >>> grub_util_warn ("Refusing to overwrite XFS meta-data."); >>> >>> This is more informative, and with grub_util_warn() user has an opportunity >>> to >>> override it if she knows what she's doing. >>> >>> >>> >> Installing with blocklists/to partition is considered >> backward-compatibility feature. We never supported a config with XFS why >> we would want bw-compat for it? >> > > Because we can't reliably tell if it's a config with XFS, only the user can. > This is an issue for both MBR or PBR installs. > > Maybe "XFSB" is only a remnant from one of this disk / partition former > lifes. Maybe it's a valid XFS but user no longer cares about it. Or > maybe a DOS-style label was created on top of it, without overwriting the > first > 440 bytes. Or maybe another filesystem had overwritten most XFS metadata > but preserved the first block (this is conceivable since other filesystems > tend to avoid using the first block). > > If user has to workaround GRUB heuristics by dd'ing zeros into a partition > before running grub-install, this is a sign GRUB isn't doing the right thing. > > Well, ok. But then I would ask to use a separate --force e.g. --force-destroy-xfs since users and distributions tend to use --force too much -- Regards Vladimir 'phcoder' Serbinenko Personal git repository: http://repo.or.cz/w/grub2/phcoder.git ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Refuse to install on XFS destroying its superblock
On Fri, Oct 16, 2009 at 10:09:41PM +0200, Vladimir 'phcoder' Serbinenko wrote: > Robert Millan wrote: > > On Fri, Oct 16, 2009 at 06:01:56PM +0200, Jordi Mallach wrote: > > > >> On Fri, Oct 16, 2009 at 04:03:01PM +0200, Vladimir 'phcoder' Serbinenko > >> wrote: > >> > >>> + if (memcmp (tmp_img, "XFSB", 4) == 0) > >>> +grub_util_error ("Can't install on XFS."); > >>> > >> Can this error message give some more detail on what the problem is? > >> > > > > I suggest something like: > > > > grub_util_warn ("Refusing to overwrite XFS meta-data."); > > > > This is more informative, and with grub_util_warn() user has an opportunity > > to > > override it if she knows what she's doing. > > > > > Installing with blocklists/to partition is considered > backward-compatibility feature. We never supported a config with XFS why > we would want bw-compat for it? Because we can't reliably tell if it's a config with XFS, only the user can. This is an issue for both MBR or PBR installs. Maybe "XFSB" is only a remnant from one of this disk / partition former lifes. Maybe it's a valid XFS but user no longer cares about it. Or maybe a DOS-style label was created on top of it, without overwriting the first 440 bytes. Or maybe another filesystem had overwritten most XFS metadata but preserved the first block (this is conceivable since other filesystems tend to avoid using the first block). If user has to workaround GRUB heuristics by dd'ing zeros into a partition before running grub-install, this is a sign GRUB isn't doing the right thing. -- Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all." ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Refuse to install on XFS destroying its superblock
Robert Millan wrote: > On Fri, Oct 16, 2009 at 06:01:56PM +0200, Jordi Mallach wrote: > >> On Fri, Oct 16, 2009 at 04:03:01PM +0200, Vladimir 'phcoder' Serbinenko >> wrote: >> >>> + if (memcmp (tmp_img, "XFSB", 4) == 0) >>> +grub_util_error ("Can't install on XFS."); >>> >> Can this error message give some more detail on what the problem is? >> > > I suggest something like: > > grub_util_warn ("Refusing to overwrite XFS meta-data."); > > This is more informative, and with grub_util_warn() user has an opportunity to > override it if she knows what she's doing. > > Installing with blocklists/to partition is considered backward-compatibility feature. We never supported a config with XFS why we would want bw-compat for it? -- Regards Vladimir 'phcoder' Serbinenko Personal git repository: http://repo.or.cz/w/grub2/phcoder.git ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Refuse to install on XFS destroying its superblock
On Fri, Oct 16, 2009 at 06:01:56PM +0200, Jordi Mallach wrote: > On Fri, Oct 16, 2009 at 04:03:01PM +0200, Vladimir 'phcoder' Serbinenko wrote: > > + if (memcmp (tmp_img, "XFSB", 4) == 0) > > +grub_util_error ("Can't install on XFS."); > > Can this error message give some more detail on what the problem is? I suggest something like: grub_util_warn ("Refusing to overwrite XFS meta-data."); This is more informative, and with grub_util_warn() user has an opportunity to override it if she knows what she's doing. -- Robert Millan The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and how) you may access your data; but nobody's threatening your freedom: we still allow you to remove your data and not access it at all." ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Refuse to install on XFS destroying its superblock
On Fri, Oct 16, 2009 at 04:03:01PM +0200, Vladimir 'phcoder' Serbinenko wrote: > + if (memcmp (tmp_img, "XFSB", 4) == 0) > +grub_util_error ("Can't install on XFS."); Can this error message give some more detail on what the problem is? Jordi -- Jordi Mallach Pérez -- Debian developer http://www.debian.org/ jo...@sindominio.net jo...@debian.org http://www.sindominio.net/ GnuPG public key information available at http://oskuro.net/ ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Refuse to install on XFS destroying its superblock
Sorry, patch had a problem Vladimir 'phcoder' Serbinenko wrote: -- Regards Vladimir 'phcoder' Serbinenko Personal git repository: http://repo.or.cz/w/grub2/phcoder.git diff --git a/ChangeLog b/ChangeLog index b0864a9..a67fdfd 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +2009-10-16 Vladimir Serbinenko + + * util/i386/pc/grub-setup.c (setup): Refuse to overwrite XFS superblock. + 2009-10-15 Vladimir Serbinenko * loader/i386/pc/xnu.c (grub_xnu_set_video): Fix loading splash image. diff --git a/util/i386/pc/grub-setup.c b/util/i386/pc/grub-setup.c index ccfbd1d..b3f2736 100644 --- a/util/i386/pc/grub-setup.c +++ b/util/i386/pc/grub-setup.c @@ -251,6 +251,9 @@ setup (const char *dir, if (grub_disk_read (dest_dev->disk, 0, 0, GRUB_DISK_SECTOR_SIZE, tmp_img)) grub_util_error ("%s", grub_errmsg); + if (memcmp (tmp_img, "XFSB", 4) == 0) +grub_util_error ("Can't install on XFS."); + /* Copy the possible DOS BPB. */ memcpy (boot_img + GRUB_BOOT_MACHINE_BPB_START, tmp_img + GRUB_BOOT_MACHINE_BPB_START, ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] Refuse to install on XFS destroying its superblock
-- Regards Vladimir 'phcoder' Serbinenko Personal git repository: http://repo.or.cz/w/grub2/phcoder.git diff --git a/ChangeLog b/ChangeLog index b0864a9..a67fdfd 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +2009-10-16 Vladimir Serbinenko + + * util/i386/pc/grub-setup.c (setup): Refuse to overwrite XFS superblock. + 2009-10-15 Vladimir Serbinenko * loader/i386/pc/xnu.c (grub_xnu_set_video): Fix loading splash image. diff --git a/util/i386/pc/grub-setup.c b/util/i386/pc/grub-setup.c index ccfbd1d..5181e58 100644 --- a/util/i386/pc/grub-setup.c +++ b/util/i386/pc/grub-setup.c @@ -205,6 +205,9 @@ setup (const char *dir, boot_img = grub_util_read_image (boot_path); free (boot_path); + if (memcmp (boot_img, "XFSB", 4) == 0) +grub_util_error ("Can't install on XFS."); + /* Set the addresses of variables in the boot image. */ boot_drive = (grub_uint8_t *) (boot_img + GRUB_BOOT_MACHINE_BOOT_DRIVE); kernel_sector = (grub_disk_addr_t *) (boot_img ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel