Re: [PATCH 2/2] util: confirm directory creation in grub_install_mkdir_p

[Darren Kenny]

Hi Vladimir,

I admit, that I did not previously.

I just tried it out where one of the components in a deeper path was a
symlink, and it works as I would expect. It will see it as a
directory in grub_util_is_directory() using stat(), since that is
looking at what is pointed to, in comparison to lstat(), which only
looks at the symlink itself.

Thanks,

Darren.

On Tuesday, 2022-08-09 at 16:04:32 +02, Vladimir Serbinenko wrote:
> Did you test the case when some of components exist and are symlinks? E.g.
> /temp being a symlinkto /var/tmp
>
> Le mar. 9 août 2022, 15:30, Darren Kenny <darren.kenny@oracle.com> a écrit :
>
>> Because grub_util_mkdir() is implemented to not return a value on any
>> platform, grub_instal_mkdir_p can test for success by confirming that
>> the directory requested exists after attempting to create it, otherwise
>> it should fail with an error and exit.
>>
>> While fixing this, a flaw in the logic was shown, where the first match
>> of the path separator, which almost always was the first character in
>> the path (e.g. /boot/grub2) would result in creating a directory with an
>> empty name (i.e. ""). To avoid that, it should skip the handling of the
>> path separator where p is pointing to the first character.
>>
>> Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
>> ---
>>  util/grub-install-common.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/util/grub-install-common.c b/util/grub-install-common.c
>> index 347558bf5412..035293c2357e 100644
>> --- a/util/grub-install-common.c
>> +++ b/util/grub-install-common.c
>> @@ -173,15 +173,20 @@ grub_install_mkdir_p (const char *dst)
>>    char *p;
>>    for (p = t; *p; p++)
>>      {
>> -      if (is_path_separator (*p))
>> +      if (is_path_separator (*p) && p != t)
>>         {
>>           char s = *p;
>>           *p = '\0';
>>           grub_util_mkdir (t);
>> +         if (!grub_util_is_directory(t))
>> +           grub_util_error (_("failed to make directory: '%s'"), t);
>> +
>>           *p = s;
>>         }
>>      }
>>    grub_util_mkdir (t);
>> +  if (!grub_util_is_directory(t))
>> +    grub_util_error (_("failed to make directory: '%s'"), t);
>>    free (t);
>>  }
>>
>> --
>> 2.31.1
>>
>>
>> _______________________________________________
>> Grub-devel mailing list
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel