[PATCH] mkfs.ubifs: Add support for symlinks in device table

[PATCH] mkfs.ubifs: Add support for symlinks in device table

[David Engraf]

It is not possible to change permissions and ownership for symlinks in the
device table file. This patch adds support for symlinks equal to mkfs.jffs2
and updates the sample device table file.

Signed-off-by: David Engraf <david.engraf@sysgo.com>
---
 jffsX-utils/device_table.txt      | 1 +
 ubifs-utils/mkfs.ubifs/devtable.c | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/jffsX-utils/device_table.txt b/jffsX-utils/device_table.txt
index 394a62b..8930cae 100644
--- a/jffsX-utils/device_table.txt
+++ b/jffsX-utils/device_table.txt
@@ -14,6 +14,7 @@
 #	c	Character special device file
 #	b	Block special device file
 #	p	Fifo (named pipe)
+#	l	Link
 # uid is the user id for the target file, gid is the group id for the
 # target file.  The rest of the entried apply only to device special
 # file.
diff --git a/ubifs-utils/mkfs.ubifs/devtable.c b/ubifs-utils/mkfs.ubifs/devtable.c
index dee035d..c96de53 100644
--- a/ubifs-utils/mkfs.ubifs/devtable.c
+++ b/ubifs-utils/mkfs.ubifs/devtable.c
@@ -33,6 +33,7 @@
  * c  Character special device file
  * b  Block special device file
  * p  Fifo (named pipe)
+ * l  Link
  *
  * Don't bother with symlinks (permissions are irrelevant), hard links (special
  * cases of regular files), or sockets (why bother).
@@ -177,6 +178,9 @@ static int interpret_table_entry(const char *line)
 		case 'b':
 			mode |= S_IFBLK;
 			break;
+		case 'l':
+			mode |= S_IFLNK;
+			break;
 		default:
 			return err_msg("unsupported file type '%c'", type);
 	}
-- 
2.11.0

Re: [PATCH] mkfs.ubifs: Add support for symlinks in device table

[Richard Weinberger]

David,

On Thu, May 18, 2017 at 3:38 PM, David Engraf <david.engraf@sysgo.com> wrote:
> It is not possible to change permissions and ownership for symlinks in the
> device table file. This patch adds support for symlinks equal to mkfs.jffs2
> and updates the sample device table file.

I'm confused, permissions of a symlink are irrelevant.
What is the usecase?

-- 
Thanks,
//richard

Re: [PATCH] mkfs.ubifs: Add support for symlinks in device table

[David Engraf]

Hi Richard,

Am 18.05.2017 um 16:33 schrieb Richard Weinberger:
> David,
> 
> On Thu, May 18, 2017 at 3:38 PM, David Engraf <david.engraf@sysgo.com> wrote:
>> It is not possible to change permissions and ownership for symlinks in the
>> device table file. This patch adds support for symlinks equal to mkfs.jffs2
>> and updates the sample device table file.
> 
> I'm confused, permissions of a symlink are irrelevant.
> What is the usecase?
> 

You're right the permissions are ignored for symlinks but the user and 
group id is used. Without this flag mkfs.ubifs takes uid/gid from the 
local system.

Best regards
- David

Re: [PATCH] mkfs.ubifs: Add support for symlinks in device table

[Richard Weinberger]

David,

Am 18.05.2017 um 16:38 schrieb David Engraf:
>> I'm confused, permissions of a symlink are irrelevant.
>> What is the usecase?
>>
> 
> You're right the permissions are ignored for symlinks but the user and group id is used. Without this flag mkfs.ubifs takes uid/gid from the local system.

IMHO we can just force the uid/gid to 0, no?

Thanks,
//richard

Re: [PATCH] mkfs.ubifs: Add support for symlinks in device table

[David Engraf]

Hi Richard,

Am 18.05.2017 um 16:47 schrieb Richard Weinberger:
> David,
> 
> Am 18.05.2017 um 16:38 schrieb David Engraf:
>>> I'm confused, permissions of a symlink are irrelevant.
>>> What is the usecase?
>>>
>>
>> You're right the permissions are ignored for symlinks but the user and group id is used. Without this flag mkfs.ubifs takes uid/gid from the local system.
> 
> IMHO we can just force the uid/gid to 0, no?

Why? It doesn't make sense to have all uid/gid to 0. This should be 
decided by the user. The purpose of the device table file is to allow 
the user to specify the permissions/uid/gid to his requirements. 
mkfs.jffs2 handles it like that as well.

Best regards
- David

Re: [PATCH] mkfs.ubifs: Add support for symlinks in device table

[Richard Weinberger]

David,

Am 18.05.2017 um 17:03 schrieb David Engraf:
>>> You're right the permissions are ignored for symlinks but the user and group id is used. Without this flag mkfs.ubifs takes uid/gid from the local system.
>>
>> IMHO we can just force the uid/gid to 0, no?
> 
> Why? It doesn't make sense to have all uid/gid to 0. This should be decided by the user. The purpose of the device table file is to allow the user to specify the
> permissions/uid/gid to his requirements. mkfs.jffs2 handles it like that as well.

Since the ownership is ignored too, what is the usecase?

Thanks,
//richard

Re: [PATCH] mkfs.ubifs: Add support for symlinks in device table

[David Engraf]

Hi Richard,

Am 18.05.2017 um 17:08 schrieb Richard Weinberger:
> David,
> 
> Am 18.05.2017 um 17:03 schrieb David Engraf:
>>>> You're right the permissions are ignored for symlinks but the user and group id is used. Without this flag mkfs.ubifs takes uid/gid from the local system.
>>>
>>> IMHO we can just force the uid/gid to 0, no?
>>
>> Why? It doesn't make sense to have all uid/gid to 0. This should be decided by the user. The purpose of the device table file is to allow the user to specify the
>> permissions/uid/gid to his requirements. mkfs.jffs2 handles it like that as well.
> 
> Since the ownership is ignored too, what is the usecase?

The ownership is not ignored, you can have a different one for a symlink 
(check "chown -h"). This is supported on ubifs:

lrwxrwxrwx    1 1        1     9 Jan  1 00:00 link -> orig_file
-rw-r--r--    1 0        0     0 Jan  1 00:00 orig_file

Best regards
- David

Re: [PATCH] mkfs.ubifs: Add support for symlinks in device table

[Richard Weinberger]

David,

Am 18.05.2017 um 17:16 schrieb David Engraf:
> Hi Richard,
> 
> Am 18.05.2017 um 17:08 schrieb Richard Weinberger:
>> David,
>>
>> Am 18.05.2017 um 17:03 schrieb David Engraf:
>>>>> You're right the permissions are ignored for symlinks but the user and group id is used. Without this flag mkfs.ubifs takes uid/gid from the local system.
>>>>
>>>> IMHO we can just force the uid/gid to 0, no?
>>>
>>> Why? It doesn't make sense to have all uid/gid to 0. This should be decided by the user. The purpose of the device table file is to allow the user to specify the
>>> permissions/uid/gid to his requirements. mkfs.jffs2 handles it like that as well.
>>
>> Since the ownership is ignored too, what is the usecase?
> 
> The ownership is not ignored, you can have a different one for a symlink (check "chown -h"). This is supported on ubifs:
> 
> lrwxrwxrwx    1 1        1     9 Jan  1 00:00 link -> orig_file
> -rw-r--r--    1 0        0     0 Jan  1 00:00 orig_file

Ignored in terms of not evaluated.

i.e.
rw@captainhero:/mnt/dir> id
uid=1000(rw) gid=100(users) groups=100(users)
rw@captainhero:/mnt/dir> ls -la
total 0
drwxr-xr-x 2 rw   root 240 May 18 17:23 .
drwxr-xr-x 3 root root 224 May 18 17:22 ..
lrwxrwxrwx 1 root root  11 May 18 17:23 s -> /etc/shadow
rw@captainhero:/mnt/dir> rm s
rw@captainhero:/mnt/dir> ls -la
total 0
drwxr-xr-x 2 rw   root 176 May 18 17:24 .
drwxr-xr-x 3 root root 224 May 18 17:22 ..
rw@captainhero:/mnt/dir>

Although, user rw is not root, it can delete "s" which is owned by root
because rw has write permission on /mnt/dir.

That's why I'm interested in the usecase.

Thanks,
//richard

Re: [PATCH] mkfs.ubifs: Add support for symlinks in device table

[Richard Weinberger]

Am 18.05.2017 um 17:27 schrieb Richard Weinberger:
> i.e.
> rw@captainhero:/mnt/dir> id
> uid=1000(rw) gid=100(users) groups=100(users)
> rw@captainhero:/mnt/dir> ls -la
> total 0
> drwxr-xr-x 2 rw   root 240 May 18 17:23 .
> drwxr-xr-x 3 root root 224 May 18 17:22 ..
> lrwxrwxrwx 1 root root  11 May 18 17:23 s -> /etc/shadow
> rw@captainhero:/mnt/dir> rm s
> rw@captainhero:/mnt/dir> ls -la
> total 0
> drwxr-xr-x 2 rw   root 176 May 18 17:24 .
> drwxr-xr-x 3 root root 224 May 18 17:22 ..
> rw@captainhero:/mnt/dir>
> 
> Although, user rw is not root, it can delete "s" which is owned by root
> because rw has write permission on /mnt/dir.

BTW: My example is misleading. Same applies to regular files.
Permissions and ownership of symlinks just don't apply.

Do you only care about the ls output or does one of your applications
depend on it?

Thanks,
//richard

Re: [PATCH] mkfs.ubifs: Add support for symlinks in device table

[David Engraf]

Hi Richard,

Am 18.05.2017 um 17:43 schrieb Richard Weinberger:
> Am 18.05.2017 um 17:27 schrieb Richard Weinberger:
>> i.e.
>> rw@captainhero:/mnt/dir> id
>> uid=1000(rw) gid=100(users) groups=100(users)
>> rw@captainhero:/mnt/dir> ls -la
>> total 0
>> drwxr-xr-x 2 rw   root 240 May 18 17:23 .
>> drwxr-xr-x 3 root root 224 May 18 17:22 ..
>> lrwxrwxrwx 1 root root  11 May 18 17:23 s -> /etc/shadow
>> rw@captainhero:/mnt/dir> rm s
>> rw@captainhero:/mnt/dir> ls -la
>> total 0
>> drwxr-xr-x 2 rw   root 176 May 18 17:24 .
>> drwxr-xr-x 3 root root 224 May 18 17:22 ..
>> rw@captainhero:/mnt/dir>
>>
>> Although, user rw is not root, it can delete "s" which is owned by root
>> because rw has write permission on /mnt/dir.
> 
> BTW: My example is misleading. Same applies to regular files.
> Permissions and ownership of symlinks just don't apply.
> 
> Do you only care about the ls output or does one of your applications
> depend on it?

It's mainly the output of ls because I'm using a verify script checking 
ownership of each file. Without having the ability to set the ownership 
of symlinks it will fail because ls returns a uid/gid of my host 
development system which is not valid on the target.

Best regards
- David

Re: [PATCH] mkfs.ubifs: Add support for symlinks in device table

[Richard Weinberger]

David,

Am 19.05.2017 um 08:13 schrieb David Engraf:
>> Do you only care about the ls output or does one of your applications
>> depend on it?
> 
> It's mainly the output of ls because I'm using a verify script checking ownership of each file. Without having the ability to set the ownership of symlinks it will fail because ls
> returns a uid/gid of my host development system which is not valid on the target.

this makes sense.

Please also massage this comment in devtable.c:
 * Don't bother with symlinks (permissions are irrelevant), hard links (special
 * cases of regular files), or sockets (why bother).

and make sure that the permissions are not set for symlinks.
Otherwise I'd give users the impression that symlinks have permissions.

Thanks,
//richard

Re: [PATCH] mkfs.ubifs: Add support for symlinks in device table

[David Engraf]

Hi Richard,

Am 19.05.2017 um 10:24 schrieb Richard Weinberger:
> David,
> 
> Am 19.05.2017 um 08:13 schrieb David Engraf:
>>> Do you only care about the ls output or does one of your applications
>>> depend on it?
>>
>> It's mainly the output of ls because I'm using a verify script checking ownership of each file. Without having the ability to set the ownership of symlinks it will fail because ls
>> returns a uid/gid of my host development system which is not valid on the target.
> 
> this makes sense.
> 
> Please also massage this comment in devtable.c:
>   * Don't bother with symlinks (permissions are irrelevant), hard links (special
>   * cases of regular files), or sockets (why bother).

Okay.

> and make sure that the permissions are not set for symlinks.
> Otherwise I'd give users the impression that symlinks have permissions.

You mean the mode should be ignore in the device table for symlinks. 
This means the mode is taken from the original file and stored in the 
ubifs inode (add_inode()). This would be okay for me but what is ubifs 
doing with this information stored in its inode?

We could also force symlinks to 777 and return an error if the user 
tries to use a different value. What do you think?

Best regards
- David

Re: [PATCH] mkfs.ubifs: Add support for symlinks in device table

[Richard Weinberger]

David,

Am 19.05.2017 um 10:47 schrieb David Engraf:
> We could also force symlinks to 777 and return an error if the user tries to use a different value. What do you think?

Exactly. :-)

Thanks,
//richard

[PATCH v2] mkfs.ubifs: Add support for symlinks in device table

[David Engraf]

It is not possible to change ownership for symlinks in the device table file.
This patch adds support for symlinks equal to mkfs.jffs2 and updates the
sample device table file. The permission entry for symlinks in the device
table must be set to 0777.

Signed-off-by: David Engraf <david.engraf@sysgo.com>
---
 jffsX-utils/device_table.txt      |  1 +
 ubifs-utils/mkfs.ubifs/devtable.c | 10 ++++++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/jffsX-utils/device_table.txt b/jffsX-utils/device_table.txt
index 394a62b..8930cae 100644
--- a/jffsX-utils/device_table.txt
+++ b/jffsX-utils/device_table.txt
@@ -14,6 +14,7 @@
 #	c	Character special device file
 #	b	Block special device file
 #	p	Fifo (named pipe)
+#	l	Link
 # uid is the user id for the target file, gid is the group id for the
 # target file.  The rest of the entried apply only to device special
 # file.
diff --git a/ubifs-utils/mkfs.ubifs/devtable.c b/ubifs-utils/mkfs.ubifs/devtable.c
index dee035d..3b46abc 100644
--- a/ubifs-utils/mkfs.ubifs/devtable.c
+++ b/ubifs-utils/mkfs.ubifs/devtable.c
@@ -33,9 +33,10 @@
  * c  Character special device file
  * b  Block special device file
  * p  Fifo (named pipe)
+ * l  Link
  *
- * Don't bother with symlinks (permissions are irrelevant), hard links (special
- * cases of regular files), or sockets (why bother).
+ * Don't bother with hard links (special cases of regular files), or sockets
+ * (why bother).
  *
  * Regular files must exist in the target root directory. If a char, block,
  * fifo, or directory does not exist, it will be created.
@@ -177,6 +178,11 @@ static int interpret_table_entry(const char *line)
 		case 'b':
 			mode |= S_IFBLK;
 			break;
+		case 'l':
+			mode |= S_IFLNK;
+			if ((mode & 0777) != 0777)
+				return err_msg("link permission must be 0777");
+			break;
 		default:
 			return err_msg("unsupported file type '%c'", type);
 	}
-- 
2.11.0

Re: [PATCH v2] mkfs.ubifs: Add support for symlinks in device table

[David Oberhollenzer]

Applied to mtd-utils.git.

Thanks,

David

On 05/23/2017 08:52 AM, David Engraf wrote:
> It is not possible to change ownership for symlinks in the device table file.
> This patch adds support for symlinks equal to mkfs.jffs2 and updates the
> sample device table file. The permission entry for symlinks in the device
> table must be set to 0777.
> 
> Signed-off-by: David Engraf <david.engraf@sysgo.com>
> ---
>  jffsX-utils/device_table.txt      |  1 +
>  ubifs-utils/mkfs.ubifs/devtable.c | 10 ++++++++--
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/jffsX-utils/device_table.txt b/jffsX-utils/device_table.txt
> index 394a62b..8930cae 100644
> --- a/jffsX-utils/device_table.txt
> +++ b/jffsX-utils/device_table.txt
> @@ -14,6 +14,7 @@
>  #	c	Character special device file
>  #	b	Block special device file
>  #	p	Fifo (named pipe)
> +#	l	Link
>  # uid is the user id for the target file, gid is the group id for the
>  # target file.  The rest of the entried apply only to device special
>  # file.
> diff --git a/ubifs-utils/mkfs.ubifs/devtable.c b/ubifs-utils/mkfs.ubifs/devtable.c
> index dee035d..3b46abc 100644
> --- a/ubifs-utils/mkfs.ubifs/devtable.c
> +++ b/ubifs-utils/mkfs.ubifs/devtable.c
> @@ -33,9 +33,10 @@
>   * c  Character special device file
>   * b  Block special device file
>   * p  Fifo (named pipe)
> + * l  Link
>   *
> - * Don't bother with symlinks (permissions are irrelevant), hard links (special
> - * cases of regular files), or sockets (why bother).
> + * Don't bother with hard links (special cases of regular files), or sockets
> + * (why bother).
>   *
>   * Regular files must exist in the target root directory. If a char, block,
>   * fifo, or directory does not exist, it will be created.
> @@ -177,6 +178,11 @@ static int interpret_table_entry(const char *line)
>  		case 'b':
>  			mode |= S_IFBLK;
>  			break;
> +		case 'l':
> +			mode |= S_IFLNK;
> +			if ((mode & 0777) != 0777)
> +				return err_msg("link permission must be 0777");
> +			break;
>  		default:
>  			return err_msg("unsupported file type '%c'", type);
>  	}
>