Re: ext4: Funny characters appended to file names
From: Dimitri John Ledkov <hidden>
Date: 2020-12-07 02:01:41
Hi, On Sun, 6 Dec 2020, 18:28 Colin Watson, [off-list ref] wrote:
On Sun, Dec 06, 2020 at 02:44:16PM +0000, Colin Watson wrote:quoted
So, in the RESTORE_BACKUP case, shouldn't that be: char *dstf = grub_util_path_concat (2, di, de->d_name); ... rather than grub_util_path_concat_ext? Otherwise it seems to me that it's going to try to append an additional argument which doesn't exist, and may well add random uninitialised stuff from memory. Running grub-install under valgrind would probably show this up (I can't get it to do it for me so far, but most likely I just haven't set up quite the right initial conditions).While I couldn't reproduce this on amd64 (and valgrind didn't show any errors), I can reproduce it just fine on i386, which is what Paul is using. I guess the va_list layout in memory is different enough between the two ABIs to tickle this.
Indeed, my patch was not tested on i386 and has had limited reviews before landing in Ubuntu. Still not reviewed on the upstream mailing list. I am sorry for causing this issue.
quoted hunk ↗ jump to hunk
I'll apply this patch for Debian grub2 2.04-11, which I've confirmed fixes it for me:diff --git a/util/grub-install-common.c b/util/grub-install-common.c index a883b6dae..61f9075bc 100644 --- a/util/grub-install-common.c +++ b/util/grub-install-common.c@@ -247,7 +247,7 @@ clean_grub_dir_real (const char *di, enum clean_grub_dir_mode mode) } else if (mode == RESTORE_BACKUP) { - char *dstf = grub_util_path_concat_ext (2, di, de->d_name); + char *dstf = grub_util_path_concat (2, di, de->d_name); dstf[strlen (dstf) - 1] = 0; if (grub_util_rename (srcf, dstf) < 0) grub_util_error (_("cannot restore `%s': %s"), dstf,Dimitri, I know Ubuntu isn't very interested in supporting i386, but IMO you should apply this patch to Ubuntu in any case; it might affect other architectures, and anyway leaving known undefined behaviour around isn't a good idea.
Will do. And will resend it to the grub mailing list.
Paul, note that you'll also need to run "sudo rm -f
/boot/grub/i386-pc/*.{img,lst,mo,mod,o,sh}-*" to clean up the stray
files created by this bug.Thank you everyone for the code review.