On Mon, May 24, 2021 at 10:59:34AM +0100, Dimitri John Ledkov wrote:
> From: Dimitri John Ledkov <x...@ubuntu.com>
>
> Refactor clean_grub_dir to create a backup of all the files, instead
> of just irrevocably removing them as the first action. If available,
> register atexit handle to restore the backup if errors occur before
> point of no return, or remove the backup if everything was
> successful. If atexit is not available, the backup remains on disk for
> manual recovery.
>
> Some platforms defined a point of no return, i.e. after modules & core
> images were updated. Failures from any commands after that stage are
> ignored, and backup is cleanedup. For example, on EFI platforms update
> is not reverted when efibootmgr fails.
>
> Extra care is taken to ensure atexit handler is only invoked by the
> parent process and not any children forks. Some older grub codebases
> can invoke parent atexit hooks from forks, which can mess up the
> backup.
>
> This allows safer upgrades of MBR & modules, such that
> modules/images/fonts/translations are consistent with MBR in case of
> errors. For example accidental grub-install /dev/non-existent-disk
> currently clobbers and upgrades modules in /boot/grub, despite not
> actually updating any MBR.
>
> This patch only handles backup and restore of files copied to
> /boot/grub. This patch does not perform backup (or restoration) of MBR
> itself or blocklists. Thus when installing i386-pc platform,
> corruption may still occur with MBR and blocklists which will not be
> attempted to be automatically recovered.
>
> Also add modinfo.sh and *.efi to the cleanup/backup/restore codepath,
> to ensure it is also cleaned / backed up / restored.
>
> Signed-off-by: Dimitri John Ledkov <x...@ubuntu.com>
Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com>
But...
> +static void
> +append_to_backup_dirs (const char *dir)
> +{
> + backup_dirs = xrealloc (backup_dirs, sizeof (char *) * (backup_dirs_size +
> 1));
> + backup_dirs[backup_dirs_size] = strdup (dir);
s/strdup/xstrdup/
I will fix it before committing...
> + backup_dirs_size++;
> + if (!backup_process)
> + {
> + atexit (restore_backup_atexit);
> + backup_process = getpid ();
> + }
> +}
Thank you for doing the work!
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
