Re: mkfs.ubifs issues with mtd-utils 2.1.0

[David Oberhollenzer <david.oberhollenzer@xxxxxxxxxxxxx>]

Hi,

On 3/30/19 4:43 AM, Markus Mayer wrote:
> The code that's complaining:
> 
> ./ubifs-utils/mkfs.ubifs/mkfs.ubifs.c
> 1533         if (data_len) {
> 1534                 if (!S_ISLNK(st->st_mode))
> 1535                         return err_msg("Expected symlink");
> 
> Clearly, checking just for symlinks at this points seems to be too
> restrictive. /dev/console is a character device, and there is no
> reason why mkfs.ubifs should refuse to create it. After all, it has
> always worked in the past.

This check was introduced in "a767dd30 mkfs.ubifs: Add encrypted symlink support".

This commit is part of the fscrypt patch set. A few lines below, the extra data is
encrypted with the path encryption functions if there is an fscrypt context.

Given that having device files on disk outside an initramfs is a rather rare thing
nowadays (devtmpfs), it appears we unfortunately didn't think of that case and
tests subsequently didn't cover it either.

> Since I am not entirely sure of the purpose of the check, I am also
> unsure what solution to propose. Would
>     if (S_ISREG(st->st_mode)) /* error here */
> or
>     if (S_ISREG(st->st_mode) || S_ISDIR(st->st_mode)) /* error here */
> be sufficient? This would allow links, character & block devices, sockets, etc.

A quick and dirty fix would be to move the check into the branch where the data
is actually encrypted.

However, the question remains what the fscrypt branch should actually do with
a device number in this case.

If the user space tooling made the implicit assumption that it must be a symlink
if it has extra data, chances are that the corresponding UBIFS kernel patch set
makes the same assumption.

Thanks for reporting this,

David

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/