[PATCH 0/6] fix xuser write to usb storage failed

[PATCH 0/6] fix xuser write to usb storage failed

[Hongxu Jia]

Test Case

* Steps
1) preparation
...
one target: atom-pc
two usb sticks: one for boot and install, another for plug test, they are both vfat filesystem.
...

2) config 
conf/local.conf:
...
MACHINE ?= "atom-pc"
...

3) build image
bitbake core-image-sato

4) load image to atom-pc
Test Case TC-2927: boot and install from usb 

5) open a terminal/ssh of atom-pc
Test Case TC-2955: remote access by ssh 

6) hotplug test: usb stick with vfat filesystem is accessible
Test Case TC-2947: usb mount
Test Case TC-2950: usb write files
Test Case TC-2948: usb read files
Test Case TC-2949: usb umount

7) coldplug test: usb stick with vfat filesystem is accessible
plug usb stick
boot system
create files in usb
view files in usb by file browser

8) use systemd for the main image and use sysvinit for the rescue image
Modify conf/local.conf, and repeat step 3 to 7;
...
DISTRO_FEATURES_append = " systemd"
VIRTUAL-RUNTIME_init_manager = "systemd"
...

9) use systemd exclusively
Modify conf/local.conf, and repeat step 3 to 7;
...
DISTRO_FEATURES_append = " systemd"
VIRTUAL-RUNTIME_init_manager = "systemd"
DISTRO_FEATURES_BACKFILL_CONSIDERED = "sysvinit"
...

* Expected Results:
1) build image success

2) the owner of plugin usb stick is `xuser', it will be automatically
mount/umount, and xuser could read files from usb and write files to it.

root@emenlow-noemgd:~# mount
/dev/sdc1 on /media/sdc1 type vfat (rw,nosuid,nodev,noexec,relatime,uid=1000,gid=1000,fmask=0177,dmask=0077,codepage=437,iocharset=iso8859-1,shortname=mixed,quiet,errors=remount-ro,user)
/dev/sdb on /media/realroot type vfat (rw,nosuid,nodev,noexec,relatime,uid=1000,gid=1000,fmask=0177,dmask=0077,codepage=437,iocharset=iso8859-1,shortname=mixed,quiet,errors=remount-ro)

root@emenlow-noemgd:~# ls /media/ -al
drwxr-xr-x    2 root     root         16384 Jan  1  1970 realroot
drwx------    3 xuser    xuser         1024 Jun  9 12:08 sdc1

root@emenlow-noemgd:~# su xuser
xuser@emenlow-noemgd:~$ echo "hello world" > /media/sdc1/test
xuser@emenlow-noemgd:~$ cat /media/sdc1/test 
hello world

//Hongxu


The following changes since commit a63229917a5708de2d161aba0d67168ce0da6365:

  meta-yocto-bsp: update reference board SRCREVs (2013-07-10 09:45:51 +0100)

are available in the git repository at:

  git://git.pokylinux.org/poky-contrib hongxu/fix-udev-xuser
  http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=hongxu/fix-udev-xuser

Hongxu Jia (6):
  pmount: add version 0.9.23
  atom-pc:fix xuser write to usb storage failed
  udev-extraconf:fix xuser write to usb storage failed
  udev: fix invoking pmount failed based on sysvinit
  pmount: allow users in disk group to perform pmount/pumount
  xuser-account: add xuser to disk group

 meta-yocto-bsp/conf/machine/atom-pc.conf           |   1 +
 meta/recipes-core/udev/udev-extraconf/mount.sh     |  31 +++++-
 meta/recipes-core/udev/udev/init                   |   1 +
 ...-option-f-to-mount-already-mounted-device.patch | 115 +++++++++++++++++++++
 meta/recipes-extended/pmount/pmount_0.9.23.bb      |  34 ++++++
 .../user-creation/xuser-account_0.1.bb             |   2 +-
 6 files changed, 181 insertions(+), 3 deletions(-)
 mode change 100644 => 100755 meta/recipes-core/udev/udev-extraconf/mount.sh
 create mode 100644 meta/recipes-extended/pmount/files/pmount-add-option-f-to-mount-already-mounted-device.patch
 create mode 100644 meta/recipes-extended/pmount/pmount_0.9.23.bb

-- 
1.8.1.2

[PATCH 1/6] pmount: add version 0.9.23

[Hongxu Jia]

The pmount is a wrapper around the standard mount program which permits
normal users to mount removable devices without a matching /etc/fstab
entry.

Add option -f to mount already mounted device, while option -f is used,
if the device has already been mounted, it will umount the exist mountpoint
and then mount the current.

Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
---
 ...-option-f-to-mount-already-mounted-device.patch | 115 +++++++++++++++++++++
 meta/recipes-extended/pmount/pmount_0.9.23.bb      |  29 ++++++
 2 files changed, 144 insertions(+)
 create mode 100644 meta/recipes-extended/pmount/files/pmount-add-option-f-to-mount-already-mounted-device.patch
 create mode 100644 meta/recipes-extended/pmount/pmount_0.9.23.bb

diff --git a/meta/recipes-extended/pmount/files/pmount-add-option-f-to-mount-already-mounted-device.patch b/meta/recipes-extended/pmount/files/pmount-add-option-f-to-mount-already-mounted-device.patch
new file mode 100644
index 0000000..ca6fbbc
--- /dev/null
+++ b/meta/recipes-extended/pmount/files/pmount-add-option-f-to-mount-already-mounted-device.patch
@@ -0,0 +1,115 @@
+pmount:add option -f to mount already mounted device
+
+While option -f is used, if the device has already been mounted,
+it will umount the exist mountpoint and then mount the current.
+
+Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
+Upstream-Status: Pending
+---
+ src/pmount.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++-
+ 1 file changed, 48 insertions(+), 1 deletion(-)
+
+diff --git a/src/pmount.c b/src/pmount.c
+--- a/src/pmount.c
++++ b/src/pmount.c
+@@ -79,6 +79,8 @@ usage( const char* exename )
+     puts( _("Options:\n"
+     "  -r          : force <device> to be mounted read-only\n"
+     "  -w          : force <device> to be mounted read-write\n"
++    "  -f          : force <device> to be mounted even if it has been mounted,\n"
++    "                the previous mounted will be umounted.\n"
+     "  -s, --sync  : mount <device> with the 'sync' option (default: 'async')\n"
+     "  -A, --noatime\n"
+     "                mount <device> with the 'noatime' option (default: 'atime')\n"
+@@ -102,6 +104,31 @@ usage( const char* exename )
+ }
+ 
+ /**
++ * Check whether the given device has already been mounted.
++ * @return 1 if mounted, 0 if not mounted, -1 on failure
++ */
++int
++check_device_mounted(const char* device)
++{
++    int result = device_valid( device ) &&
++        ( device_whitelisted( device ) || device_removable( device ) ) &&
++        !device_locked( device );
++
++    if ( result )
++    {
++        result = device_mounted( device, 0, NULL );
++        if ( result )
++            debug ( "device already mounted\n" );
++    }
++    else
++    {
++        result = -1;
++        perror( _("Error: check device failed") );
++    }
++    return result;
++}
++
++/**
+  * Check whether the user is allowed to mount the given device to the given
+  * mount point. Creates the mount point if it does not exist yet. 
+  * @return 0 on success, -1 on failure
+@@ -618,6 +645,7 @@ main( int argc, char** argv )
+     int noatime = 0;
+     int exec = 0;
+     int force_write = -1; /* 0: ro, 1: rw, -1: default */
++    int force_mount = 0; /* 0: not force mount, 1: force mount */
+     const char* use_fstype = NULL;
+     const char* iocharset = NULL;
+     const char* umask = NULL;
+@@ -646,6 +674,7 @@ main( int argc, char** argv )
+         { "passphrase", 1, NULL, 'p' },
+         { "read-only", 0, NULL, 'r' },
+         { "read-write", 0, NULL, 'w' },
++        { "force-mount", 0, NULL, 'f'},
+         { "version", 0, NULL, 'V' },
+         { NULL, 0, NULL, 0}
+     };
+@@ -678,7 +707,7 @@ main( int argc, char** argv )
+ 
+     /* parse command line options */
+     do {
+-        switch( option = getopt_long( argc, argv, "+hdelLsArwp:t:c:u:V", long_opts, NULL ) ) {
++        switch( option = getopt_long( argc, argv, "+hdelLsArwfp:t:c:u:V", long_opts, NULL ) ) {
+             case -1:  break;          /* end of arguments */
+             case ':':
+             case '?': return E_ARGS;  /* unknown argument */
+@@ -713,6 +742,8 @@ main( int argc, char** argv )
+ 
+             case 'w': force_write = 1; break;
+ 
++            case 'f': force_mount = 1; break;
++
+             case 'V': puts(VERSION); return 0;
+ 
+             default:
+@@ -829,6 +860,22 @@ main( int argc, char** argv )
+             /* clean stale locks */
+             clean_lock_dir( device );
+ 
++            /* if device has already been mounted, umount it */
++            if (force_mount)
++            {
++                result = check_device_mounted(device);
++                if (result < 0)
++                    return E_POLICY;
++                else if (result == 1)
++                {
++                    debug("device alreasy mounted, umount it\n");
++                    result = spawnl( SPAWN_EROOT|SPAWN_RROOT, UMOUNTPROG, UMOUNTPROG, device, NULL );
++                    if( result != 0 ) {
++                        fprintf( stderr, _("Error: umount failed at force mount moment\n") );
++                        return -1;
++                    }
++                }
++            }
+             if( check_mount_policy( device, mntpt )  )
+                 return E_POLICY;
+ 
+-- 
+1.8.1.2
+
diff --git a/meta/recipes-extended/pmount/pmount_0.9.23.bb b/meta/recipes-extended/pmount/pmount_0.9.23.bb
new file mode 100644
index 0000000..0d357b9
--- /dev/null
+++ b/meta/recipes-extended/pmount/pmount_0.9.23.bb
@@ -0,0 +1,29 @@
+SUMMARY = "mount removable devices as normal user"
+DESCRIPTION = "pmount is a wrapper around the standard mount program which \
+permits normal users to mount removable devices without a matching /etc/fstab \
+entry. This provides a robust basis for automounting frameworks like GNOME's \
+Utopia project and confines the amount of code that runs as root to a minimum. \
+"
+HOMEPAGE = "http://pmount.alioth.debian.org/"
+SECTION = "console/utils"
+LICENSE = "GPLv3"
+LIC_FILES_CHKSUM = "file://COPYING;md5=d32239bcb673463ab874e80d47fae504"
+SRC_URI = "${DEBIAN_MIRROR}/main/p/pmount/pmount_${PV}.orig.tar.bz2 \
+          file://pmount-add-option-f-to-mount-already-mounted-device.patch \
+"
+
+DEPENDS = "glib-2.0-native intltool-native util-linux virtual/libc"
+
+# 1) Busybox's mount/umount doesn't support pmount very well, so
+# explicitly choose util-linux's mount/umount.
+# 2) It needs the existence of `/var/lock, /etc/mtab, /etc/fstab', so
+# explicitly choose base-files.
+RDEPENDS_${PN} = "util-linux-mount \
+                  util-linux-umount \
+                  base-files \
+"
+
+inherit autotools
+
+SRC_URI[md5sum] = "db19f5bf3151b1b41705ec7bafa439d3"
+SRC_URI[sha256sum] = "db38fc290b710e8e9e9d442da2fb627d41e13b3ee80326c15cc2595ba00ea036"
-- 
1.8.1.2

[PATCH 2/6] atom-pc:fix xuser write to usb storage failed

[Hongxu Jia]

Add package pmount to atom-pc, the normal user could invoke pmount to mount
storage device and would have permissions to access it.

[YOCTO #4004]

Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
---
 meta-yocto-bsp/conf/machine/atom-pc.conf | 1 +
 1 file changed, 1 insertion(+)

diff --git a/meta-yocto-bsp/conf/machine/atom-pc.conf b/meta-yocto-bsp/conf/machine/atom-pc.conf
index 6318d60..127b3ad 100644
--- a/meta-yocto-bsp/conf/machine/atom-pc.conf
+++ b/meta-yocto-bsp/conf/machine/atom-pc.conf
@@ -35,3 +35,4 @@ GLIBC_ADDONS = "nptl"
 EXTRA_OECONF_append_pn-matchbox-panel-2 = " --with-battery=acpi"
 
 ROOTLESS_X = "1"
+IMAGE_INSTALL_append = " pmount"
-- 
1.8.1.2

[PATCH 3/6] udev-extraconf:fix xuser write to usb storage failed

[Hongxu Jia]

Rootless X is owned by xuser, the problem is xuser doesn't have permissions to
write usb storage when the type of filesystem is vfat.
...
$ mount
/dev/sdb1 on /media/sdb1 type vfat
...
$ ls /media/ -al
drwxr-xr-x    2 root     root         16384 Jan  1  1970 sdb1
...

The usb storage is mounted to `/media/sd**' by a udev's mount.sh script,
modify the script to invoke pmount by xuser to resolve this issue.

[YOCTO #4004]

Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
---
 meta/recipes-core/udev/udev-extraconf/mount.sh | 31 ++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)
 mode change 100644 => 100755 meta/recipes-core/udev/udev-extraconf/mount.sh

diff --git a/meta/recipes-core/udev/udev-extraconf/mount.sh b/meta/recipes-core/udev/udev-extraconf/mount.sh
old mode 100644
new mode 100755
index d1419ed..07c8955
--- a/meta/recipes-core/udev/udev-extraconf/mount.sh
+++ b/meta/recipes-core/udev/udev-extraconf/mount.sh
@@ -8,6 +8,7 @@
 MOUNT="/bin/mount"
 PMOUNT="/usr/bin/pmount"
 UMOUNT="/bin/umount"
+PUMOUNT="/usr/bin/pumount"
 for line in `grep -v ^# /etc/udev/mount.blacklist`
 do
 	if [ ` expr match "$DEVNAME" "$line" ` -gt 0 ];
@@ -37,6 +38,28 @@ automount() {
 	fi
 }
 	
+autopmount() {
+	name="`basename "$DEVNAME"`"
+	xusername="root"
+
+	# If rootless X is enable, use a normal user (xuser) to invoke
+	# pmount to mount devices, so the user could have permissions
+	# to access the storage device.
+	if [ -f /etc/X11/Xusername ]; then
+		xusername="`cat /etc/X11/Xusername`"
+	fi
+
+	su -l -c "$PMOUNT -f $DEVNAME >/dev/null 2>&1" $xusername
+	grep -q "^$DEVNAME " /proc/mounts
+	if [ $? -ne 0 ];then
+		# logger "mount.sh/autopmount" "$PMOUNT $DEVNAME \"/media/$name\" failed!"
+		rm_dir "/media/$name"
+	else
+		logger "mount.sh/autopmount" "Auto-mount of [/media/$name] successful"
+		touch "/tmp/.automount-$name"
+	fi
+}
+
 rm_dir() {
 	# We do not want to rm -r populated directories
 	if test "`find "$1" | wc -l | tr -d " "`" -lt 2 -a -d "$1"
@@ -49,7 +72,7 @@ rm_dir() {
 
 if [ "$ACTION" = "add" ] && [ -n "$DEVNAME" ] && [ -n "$ID_FS_TYPE" ]; then
 	if [ -x "$PMOUNT" ]; then
-		$PMOUNT $DEVNAME 2> /dev/null
+		autopmount
 	elif [ -x $MOUNT ]; then
     		$MOUNT $DEVNAME 2> /dev/null
 	fi
@@ -67,7 +90,11 @@ fi
 if [ "$ACTION" = "remove" ] && [ -x "$UMOUNT" ] && [ -n "$DEVNAME" ]; then
 	for mnt in `cat /proc/mounts | grep "$DEVNAME" | cut -f 2 -d " " `
 	do
-		$UMOUNT $mnt
+		if [ -x "$PUMOUNT" ]; then
+			$PUMOUNT $mnt
+		else
+			$UMOUNT $mnt
+		fi
 	done
 	
 	# Remove empty directories from auto-mounter
-- 
1.8.1.2

[PATCH 4/6] udev: fix invoking pmount failed based on sysvinit

[Hongxu Jia]

The pmount is invoked in the udev's mount.sh script, and it needs the
existence of `/var/lock', `/var/lock' is symlinked to `/run/lock'.

When the system is based on sysvinit, `/run/lock' is created by populate-
volatile.sh, if the udev service starts before populate-volatile.sh starts,
`/var/lock' will be a invalid link.

Create `/run/lock' directory at udev startup to avoid this invalid link problem.

Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
---
 meta/recipes-core/udev/udev/init | 1 +
 1 file changed, 1 insertion(+)

diff --git a/meta/recipes-core/udev/udev/init b/meta/recipes-core/udev/udev/init
index d90d446..8e9f3f8 100644
--- a/meta/recipes-core/udev/udev/init
+++ b/meta/recipes-core/udev/udev/init
@@ -53,6 +53,7 @@ case "$1" in
     [ -e /dev/shm ] || mkdir -m 1777 /dev/shm
     mount -a -t tmpfs 2>/dev/null
     mkdir -p /var/volatile/tmp
+    mkdir -p /run/lock
 
     # Cache handling.
     # A list of files which are used as a criteria to judge whether the udev cache could be reused.
-- 
1.8.1.2

[PATCH 5/6] pmount: allow users in disk group to perform pmount/pumount

[Hongxu Jia]

* changed ownership group for /usr/bin/pmount and /usr/bin/pumount to 'disk';
* deny execution rights to other users except 'root' and those belonging
  to 'disk' group;
* set setuid bit to both apps;

So, basically, in order for a normal user to be able to pmount/pumount
the device, it must be a member of 'disk' group.

Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
---
 meta/recipes-extended/pmount/pmount_0.9.23.bb | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/meta/recipes-extended/pmount/pmount_0.9.23.bb b/meta/recipes-extended/pmount/pmount_0.9.23.bb
index 0d357b9..54504c7 100644
--- a/meta/recipes-extended/pmount/pmount_0.9.23.bb
+++ b/meta/recipes-extended/pmount/pmount_0.9.23.bb
@@ -27,3 +27,8 @@ inherit autotools
 
 SRC_URI[md5sum] = "db19f5bf3151b1b41705ec7bafa439d3"
 SRC_URI[sha256sum] = "db38fc290b710e8e9e9d442da2fb627d41e13b3ee80326c15cc2595ba00ea036"
+
+do_install_append() {
+    chown root.disk ${D}/${bindir}/pmount ${D}/${bindir}/pumount
+    chmod o-x,u+s ${D}/${bindir}/pmount ${D}/${bindir}/pumount
+}
-- 
1.8.1.2

[PATCH 6/6] xuser-account: add xuser to disk group

[Hongxu Jia]

In order for xuser to be able to perform pmount/pumount.

Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
---
 meta/recipes-support/user-creation/xuser-account_0.1.bb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/recipes-support/user-creation/xuser-account_0.1.bb b/meta/recipes-support/user-creation/xuser-account_0.1.bb
index a7575a4..58e8139 100644
--- a/meta/recipes-support/user-creation/xuser-account_0.1.bb
+++ b/meta/recipes-support/user-creation/xuser-account_0.1.bb
@@ -21,7 +21,7 @@ do_install() {
 USERADD_PACKAGES = "${PN}"
 GROUPADD_PARAM_${PN} = "--system shutdown"
 USERADD_PARAM_${PN} = "--create-home \
-                       --groups video,tty,audio,input,shutdown \
+                       --groups video,tty,audio,input,shutdown,disk \
                        --user-group xuser"
 
 ALLOW_EMPTY_${PN} = "1"
-- 
1.8.1.2

Re: [PATCH 2/6] atom-pc:fix xuser write to usb storage failed

[Saul Wold]

On 07/12/2013 02:29 AM, Hongxu Jia wrote:
> Add package pmount to atom-pc, the normal user could invoke pmount to mount
> storage device and would have permissions to access it.
>
This should really have gone to the Poky ML (poky@yoctoproject.org)

Sau!

> [YOCTO #4004]
>
> Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
> ---
>   meta-yocto-bsp/conf/machine/atom-pc.conf | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/meta-yocto-bsp/conf/machine/atom-pc.conf b/meta-yocto-bsp/conf/machine/atom-pc.conf
> index 6318d60..127b3ad 100644
> --- a/meta-yocto-bsp/conf/machine/atom-pc.conf
> +++ b/meta-yocto-bsp/conf/machine/atom-pc.conf
> @@ -35,3 +35,4 @@ GLIBC_ADDONS = "nptl"
>   EXTRA_OECONF_append_pn-matchbox-panel-2 = " --with-battery=acpi"
>
>   ROOTLESS_X = "1"
> +IMAGE_INSTALL_append = " pmount"
>

Re: [PATCH 2/6] atom-pc:fix xuser write to usb storage failed

[Hongxu Jia]

On 07/15/2013 03:14 PM, Saul Wold wrote:
> On 07/12/2013 02:29 AM, Hongxu Jia wrote:
>> Add package pmount to atom-pc, the normal user could invoke pmount to 
>> mount
>> storage device and would have permissions to access it.
>>
> This should really have gone to the Poky ML (poky@yoctoproject.org)
>
Got it, I will sent this patch to poky@yoctoproject.org
> Sau!
>
>> [YOCTO #4004]
>>
>> Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
>> ---
>>   meta-yocto-bsp/conf/machine/atom-pc.conf | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/meta-yocto-bsp/conf/machine/atom-pc.conf 
>> b/meta-yocto-bsp/conf/machine/atom-pc.conf
>> index 6318d60..127b3ad 100644
>> --- a/meta-yocto-bsp/conf/machine/atom-pc.conf
>> +++ b/meta-yocto-bsp/conf/machine/atom-pc.conf
>> @@ -35,3 +35,4 @@ GLIBC_ADDONS = "nptl"
>>   EXTRA_OECONF_append_pn-matchbox-panel-2 = " --with-battery=acpi"
>>
>>   ROOTLESS_X = "1"
>> +IMAGE_INSTALL_append = " pmount"
>>