[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Bug#681227: Can anyone reproduce #681227: installation-reports: grub-install tries to install to a nonsense string?!



tags 681227 + patch
block 651720 by 681227
# kFreeBSD bugfix couldn't enter wheezy yet due to regressions
thanks

Hi,

On 07/01/13 19:56, Wouter Verhelst wrote:
>> What to do with the workaround added by Wouter in grub-installer/1.84?
> 
> The workaround tried to eliminate the possibility of invalid data coming
> from "somewhere" in the installer. [...]

I understand this made sense *if* a bug in the installer had been
appending nonsense to an otherwise-valid $bootdev, but I think we've
disproven this now.


>> Silently ignoring a failure seems risky when we know that it should not
>> happen.  (Someone may want to specify multiple targets, and if one of
>> them is typo'd it would be silently skipped in this case).
> 
> That's indeed the only case that isn't caught by the current code.

But that was at least caught by the original code - the GRUB install
step failed if the user gave invalid input.  Except in this bug report,
the user thought the failure was a software bug, rather than wrong
keyboard input which I'm sure it was.

With the workaround still in place, it may silently ignore such an
error, whether it comes from the user or from code, and I think that is
a more harmful situation.


Removing the workaround would close regressions #696903, #696942
affecting sid, unbreaking the sid_d-i daily images, where GRUB is not
installable right now for kfreebsd-*, grub-yeeloong and apparently
grub-efi systems.

It would also allow important bugfix #681227 to migrate to testing.

IMHO it would close this bug too, because it would mean the
user-supplied bootdevs *are* being validated again.

Patch for this actually just a diff limited to ./grub-installer from:

$ git revert a070f516 99389d59 926cee22


Of course there are still ways to improve, e.g. offering a list of
partitions to choose from instead of free-text input, but anything like
that must surely wait until another release.

Thanks,
Regards,
-- 
Steven Chamberlain
steven@pyro.eu.org
diff --git a/grub-installer b/grub-installer
index 9a72e54..f01eda1 100755
--- a/grub-installer
+++ b/grub-installer
@@ -645,16 +645,11 @@ info "Installing grub on '$bootdev'"
 
 update_mtab
 
-installed=0
-
 if [ -z "$frdisk" ]; then
+
 	# Install grub on each space separated disk in the list
 	bootdevs="$bootdev"
 	for bootdev in $bootdevs; do
-		# workaround for #681227
-		if ! [ "$bootdev" = dummy -o -b "$bootdev" -o -c "$bootdev" ]; then
-			continue
-		fi
 		grub_install_params=
 		if ! is_floppy "$bootdev"; then
 			if $chroot $ROOT grub-install -h 2>&1 | grep -q no-floppy; then
@@ -690,7 +685,6 @@ if [ -z "$frdisk" ]; then
 		esac
 		if [ "$CODE" = 0 ]; then
 			info "grub-install ran successfully"
-			installed=$(( $installed + 1 ))
 		else
 			case $ARCH:$grub_package in
 			    *:grub|*:grub-pc|*:grub-efi|sparc:grub-ieee1275)
@@ -707,12 +701,7 @@ if [ -z "$frdisk" ]; then
 			exit 1
 		fi
 	done
-	if [ $installed -lt 1 ]; then
-		error "no boot device found to install to"
-		# we should probably show an error message here, but I believe
-		# we're in string freeze...
-		exit 1
-	fi
+
 else
 
 	# Semi-manual grub setup for Serial ATA RAID/multipath

Reply to: