LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* VFS and LSM issues
@ 2014-12-08 17:04 Krzysztof Opasiak
  2014-12-10  4:54 ` Greg KH
  2014-12-10 17:18 ` Stephen Smalley
  0 siblings, 2 replies; 5+ messages in thread
From: Krzysztof Opasiak @ 2014-12-08 17:04 UTC (permalink / raw)
  To: linux-usb, linux-kernel, linux-security-module, casey
  Cc: jlbec, 'Andrzej Pietrasiewicz',
	gregkh, 'Michal Nazarewicz', 'Robert Baldyga',
	Rafal Krypa, Tomasz Swierczek, 'Karol Lewandowski',
	'Marek Szyprowski', 'Stanislaw Wadas',
	kopasiak90, 'Łukasz Stelmach'

Dear All,

I'm Krzysztof Opasiak from SRPOL (Samsung). I'm working on USB support
in Tizen and mainline. In those works we use two Virtual File Systems
- ConfigFS and FunctionFS. Recently I have tried to use them with
SMACK and I ran into a few issues. Most of them looks to be a generic
problem with many FS and LSM. You can find description of those issues
and my research just below in 3 points. I'm not a VFS/LSM specialist so
your help is very welcome;)

1) Issues with function FS

It's a VFS which allow to provide custom USB function as userspace
program. I know that may be quite new for you so let's define this as
a VFS which works as follow:

$ modprobe g_ffs
$ mkdir /tmp/mount_root
$ mount none -t functionfs /tmp/mount_root
$ ls /tmp/mount_root
ep0

# now we run our program which writes some data to ep0
#  and based on this kernel creates epX
# you can find one in tools/usb/ffs-test.c

$ ./my_program /tmp/mount_root &
$ ls /tmp/mount_root
ep0 ep1 ep2

Ok so now we would like to use this together with smack. Especially
with smackfsdef mount option. First two steps go as above and then:

$ mount none -t functionfs -o smackfsdef=my_label /tmp/mount_root
$ ls -Z /tmp/mount_root/
 _ ep0

Ops! Some bug here we requested to use my_label but we got _. When we
run our program, rest of epX will get desired label (my_label). I have
started to dig in kernel to find what happen and probably I found out
where is a problem. Let's look to mount_fs() code which is executed
during mount:

struct dentry *
mount_fs(struct file_system_type *type, int flags, const char *name,
void *data)
{
(...)
	root = type->mount(type, flags, name, data);
(...)
	error = security_sb_kern_mount(sb, flags, secdata);
(...)
}

So what is important here is the order of operations. First is
executed mount ops provided by selected file system. During this mount
procedure functionfs executes new_inode(sb) to allocate inode for ep0
which should appear directly after mount. After returning from mount
function we execute security_sb_kern_mount() where we *parse the mount
options* and sets the value for lsm specific structures for example we
store the label passed in smackfsdef.

The problem here is order of calls because first we call mount for
given fs where we create a file and after this we fill security
structures with security mount options. While creating file in mount
callback super block is filled only with default values for security
so ep0 has _ label. This looks like a generic issue for all VFS which
creates indoes before or in their mount procedure.

I'm not sure if we can simply move security_sb_kern_mount() above
mount for specific fs, do we?


2) Issues with ConfigFS

ConfigFS is a generic VFS which allows to create and manage kobjects
from userspace. Each module which would like to allow for userland
driven configuration register as ConfigFS client called
subsystem. Each subsystem has its own directory in ConfigFS root
dir. We use libcomposite which appear in ConfigFS as usb_gadget
directory. In this dir you can create directories called gadgets. Some
example:

# libcomposite and configfs compiled-in
$ mount none -t configfs /sys/kernel/config
$ ls /sys/kernel/config usb_gadget
$ mkdir /sys/kernel/config/usb_gadget/g1
$ ls /sys/kernel/config/usb_gadget/g1
UDC              bDeviceSubClass  bcdUSB     idProduct  strings
bDeviceClass     bMaxPacketSize0  configs    idVendor
bDeviceProtocol  bcdDevice        functions  os_desc


Now let's try to use smack with ConfigFS. First of all we run to
similar issue as with FunctionFS so after mounting configfs with
smackfsdef option we still get _ label on subsystem directories
because they are created in configfs_register_subsystem() which is
called in libcomposite module init so really erly. So in my opinion
this looks quite similar to issue described in functionfs section.

Second issue related to Configfs is when we create the directory.
Reproduction:

$ mount none -t configfs -o smackfsdef=my_label /sys/kernel/config
$ ls -Z /sys/kernel/config
_ usb_gadget
$ chmod 777 /sys/kernel/config/usb_gadget
$ chsmack -a my_label /sys/kernel/config/usb_gadget
$ echo my_label > /proc/self/attr/current
$ su my_user
$ mkdir /sys/kernel/config/usb_gadget/g1
$ ls -Z /sys/kernel/config/usb_gadget/
_ g1

As you see our process had my_label label but created by him gadget
has the _ label and the same with all files and subdirs created in
g1. I have no idea why this happened but definetly something is
wrong. Related to this is a policy of labeling subdirectories. Obvious
thing is that g1 directory should have label different that _. But I'm
not shure what label should have its children.

3) Generic question with multi mounts filesystems

Some VFS (ConfigFS for example) can be mounted more than once. What
should be the correct behavior of smackfsdef mount option if we pass
different labels in each mount point?

Looks like all for now. I hope you were able to go through this
email without falling asleep. Thank you in advance.

--
Best regards,

Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
k.opasiak@samsung.com


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: VFS and LSM issues
  2014-12-08 17:04 VFS and LSM issues Krzysztof Opasiak
@ 2014-12-10  4:54 ` Greg KH
  2014-12-10 14:17   ` Krzysztof Opasiak
  2014-12-10 17:19   ` Stephen Smalley
  2014-12-10 17:18 ` Stephen Smalley
  1 sibling, 2 replies; 5+ messages in thread
From: Greg KH @ 2014-12-10  4:54 UTC (permalink / raw)
  To: Krzysztof Opasiak
  Cc: linux-usb, linux-kernel, linux-security-module, casey, jlbec,
	'Andrzej Pietrasiewicz', 'Michal Nazarewicz',
	'Robert Baldyga',
	Rafal Krypa, Tomasz Swierczek, 'Karol Lewandowski',
	'Marek Szyprowski', 'Stanislaw Wadas',
	kopasiak90, 'Łukasz Stelmach'

On Mon, Dec 08, 2014 at 06:04:30PM +0100, Krzysztof Opasiak wrote:
> Dear All,
> 
> I'm Krzysztof Opasiak from SRPOL (Samsung). I'm working on USB support
> in Tizen and mainline. In those works we use two Virtual File Systems
> - ConfigFS and FunctionFS. Recently I have tried to use them with
> SMACK and I ran into a few issues. Most of them looks to be a generic
> problem with many FS and LSM. You can find description of those issues
> and my research just below in 3 points. I'm not a VFS/LSM specialist so
> your help is very welcome;)
> 
> 1) Issues with function FS
> 
> It's a VFS which allow to provide custom USB function as userspace
> program. I know that may be quite new for you so let's define this as
> a VFS which works as follow:
> 
> $ modprobe g_ffs
> $ mkdir /tmp/mount_root
> $ mount none -t functionfs /tmp/mount_root
> $ ls /tmp/mount_root
> ep0
> 
> # now we run our program which writes some data to ep0
> #  and based on this kernel creates epX
> # you can find one in tools/usb/ffs-test.c
> 
> $ ./my_program /tmp/mount_root &
> $ ls /tmp/mount_root
> ep0 ep1 ep2
> 
> Ok so now we would like to use this together with smack. Especially
> with smackfsdef mount option. First two steps go as above and then:
> 
> $ mount none -t functionfs -o smackfsdef=my_label /tmp/mount_root
> $ ls -Z /tmp/mount_root/
>  _ ep0
> 
> Ops! Some bug here we requested to use my_label but we got _. When we
> run our program, rest of epX will get desired label (my_label). I have
> started to dig in kernel to find what happen and probably I found out
> where is a problem. Let's look to mount_fs() code which is executed
> during mount:
> 
> struct dentry *
> mount_fs(struct file_system_type *type, int flags, const char *name,
> void *data)
> {
> (...)
> 	root = type->mount(type, flags, name, data);
> (...)
> 	error = security_sb_kern_mount(sb, flags, secdata);
> (...)
> }
> 
> So what is important here is the order of operations. First is
> executed mount ops provided by selected file system. During this mount
> procedure functionfs executes new_inode(sb) to allocate inode for ep0
> which should appear directly after mount. After returning from mount
> function we execute security_sb_kern_mount() where we *parse the mount
> options* and sets the value for lsm specific structures for example we
> store the label passed in smackfsdef.
> 
> The problem here is order of calls because first we call mount for
> given fs where we create a file and after this we fill security
> structures with security mount options. While creating file in mount
> callback super block is filled only with default values for security
> so ep0 has _ label. This looks like a generic issue for all VFS which
> creates indoes before or in their mount procedure.
> 
> I'm not sure if we can simply move security_sb_kern_mount() above
> mount for specific fs, do we?

No, I do not think you can, sorry.

> 2) Issues with ConfigFS
> 
> ConfigFS is a generic VFS which allows to create and manage kobjects
> from userspace. Each module which would like to allow for userland
> driven configuration register as ConfigFS client called
> subsystem. Each subsystem has its own directory in ConfigFS root
> dir. We use libcomposite which appear in ConfigFS as usb_gadget
> directory. In this dir you can create directories called gadgets. Some
> example:
> 
> # libcomposite and configfs compiled-in
> $ mount none -t configfs /sys/kernel/config
> $ ls /sys/kernel/config usb_gadget
> $ mkdir /sys/kernel/config/usb_gadget/g1
> $ ls /sys/kernel/config/usb_gadget/g1
> UDC              bDeviceSubClass  bcdUSB     idProduct  strings
> bDeviceClass     bMaxPacketSize0  configs    idVendor
> bDeviceProtocol  bcdDevice        functions  os_desc
> 
> 
> Now let's try to use smack with ConfigFS. First of all we run to
> similar issue as with FunctionFS so after mounting configfs with
> smackfsdef option we still get _ label on subsystem directories
> because they are created in configfs_register_subsystem() which is
> called in libcomposite module init so really erly. So in my opinion
> this looks quite similar to issue described in functionfs section.

Yes, "virtual" filesystems like these haven't probably ever been tested
with an LSM before, as you are finding out.

> Second issue related to Configfs is when we create the directory.
> Reproduction:
> 
> $ mount none -t configfs -o smackfsdef=my_label /sys/kernel/config
> $ ls -Z /sys/kernel/config
> _ usb_gadget
> $ chmod 777 /sys/kernel/config/usb_gadget
> $ chsmack -a my_label /sys/kernel/config/usb_gadget
> $ echo my_label > /proc/self/attr/current
> $ su my_user
> $ mkdir /sys/kernel/config/usb_gadget/g1
> $ ls -Z /sys/kernel/config/usb_gadget/
> _ g1
> 
> As you see our process had my_label label but created by him gadget
> has the _ label and the same with all files and subdirs created in
> g1. I have no idea why this happened but definetly something is
> wrong. Related to this is a policy of labeling subdirectories. Obvious
> thing is that g1 directory should have label different that _. But I'm
> not shure what label should have its children.
> 
> 3) Generic question with multi mounts filesystems
> 
> Some VFS (ConfigFS for example) can be mounted more than once. What
> should be the correct behavior of smackfsdef mount option if we pass
> different labels in each mount point?

How would it be different from a "normal" filesystem you mount at
different locations?

good luck!

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: VFS and LSM issues
  2014-12-10  4:54 ` Greg KH
@ 2014-12-10 14:17   ` Krzysztof Opasiak
  2014-12-10 17:19   ` Stephen Smalley
  1 sibling, 0 replies; 5+ messages in thread
From: Krzysztof Opasiak @ 2014-12-10 14:17 UTC (permalink / raw)
  To: 'Greg KH', viro
  Cc: linux-usb, linux-kernel, linux-security-module, casey, jlbec,
	'Andrzej Pietrasiewicz', 'Michal Nazarewicz',
	'Robert Baldyga', 'Rafal Krypa',
	'Tomasz Swierczek', 'Karol Lewandowski',
	'Marek Szyprowski', 'Stanislaw Wadas',
	kopasiak90, 'Łukasz Stelmach',
	'Hillf Danton',
	chrisw, miklos



> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Wednesday, December 10, 2014 5:54 AM
> To: Krzysztof Opasiak
> Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> security-module@vger.kernel.org; casey@schaufler-ca.com;
> jlbec@evilplan.org; 'Andrzej Pietrasiewicz'; 'Michal Nazarewicz';
> 'Robert Baldyga'; Rafal Krypa; Tomasz Swierczek; 'Karol
> Lewandowski'; 'Marek Szyprowski'; 'Stanislaw Wadas';
> kopasiak90@gmail.com; 'Łukasz Stelmach'
> Subject: Re: VFS and LSM issues
> 
> On Mon, Dec 08, 2014 at 06:04:30PM +0100, Krzysztof Opasiak wrote:
> > Dear All,
> >
> > I'm Krzysztof Opasiak from SRPOL (Samsung). I'm working on USB
> support
> > in Tizen and mainline. In those works we use two Virtual File
> Systems
> > - ConfigFS and FunctionFS. Recently I have tried to use them with
> > SMACK and I ran into a few issues. Most of them looks to be a
> generic
> > problem with many FS and LSM. You can find description of those
> issues
> > and my research just below in 3 points. I'm not a VFS/LSM
> specialist so
> > your help is very welcome;)
> >
> > 1) Issues with function FS
> >
> > It's a VFS which allow to provide custom USB function as
> userspace
> > program. I know that may be quite new for you so let's define
> this as
> > a VFS which works as follow:
> >
> > $ modprobe g_ffs
> > $ mkdir /tmp/mount_root
> > $ mount none -t functionfs /tmp/mount_root
> > $ ls /tmp/mount_root
> > ep0
> >
> > # now we run our program which writes some data to ep0
> > #  and based on this kernel creates epX
> > # you can find one in tools/usb/ffs-test.c
> >
> > $ ./my_program /tmp/mount_root &
> > $ ls /tmp/mount_root
> > ep0 ep1 ep2
> >
> > Ok so now we would like to use this together with smack.
> Especially
> > with smackfsdef mount option. First two steps go as above and
> then:
> >
> > $ mount none -t functionfs -o smackfsdef=my_label /tmp/mount_root
> > $ ls -Z /tmp/mount_root/
> >  _ ep0
> >
> > Ops! Some bug here we requested to use my_label but we got _.
> When we
> > run our program, rest of epX will get desired label (my_label). I
> have
> > started to dig in kernel to find what happen and probably I found
> out
> > where is a problem. Let's look to mount_fs() code which is
> executed
> > during mount:
> >
> > struct dentry *
> > mount_fs(struct file_system_type *type, int flags, const char
> *name,
> > void *data)
> > {
> > (...)
> > 	root = type->mount(type, flags, name, data);
> > (...)
> > 	error = security_sb_kern_mount(sb, flags, secdata);
> > (...)
> > }
> >
> > So what is important here is the order of operations. First is
> > executed mount ops provided by selected file system. During this
> mount
> > procedure functionfs executes new_inode(sb) to allocate inode for
> ep0
> > which should appear directly after mount. After returning from
> mount
> > function we execute security_sb_kern_mount() where we *parse the
> mount
> > options* and sets the value for lsm specific structures for
> example we
> > store the label passed in smackfsdef.
> >
> > The problem here is order of calls because first we call mount
> for
> > given fs where we create a file and after this we fill security
> > structures with security mount options. While creating file in
> mount
> > callback super block is filled only with default values for
> security
> > so ep0 has _ label. This looks like a generic issue for all VFS
> which
> > creates indoes before or in their mount procedure.
> >
> > I'm not sure if we can simply move security_sb_kern_mount() above
> > mount for specific fs, do we?
> 
> No, I do not think you can, sorry.

That was also my opinion.

> 
> > 2) Issues with ConfigFS
> >
> > ConfigFS is a generic VFS which allows to create and manage
> kobjects
> > from userspace. Each module which would like to allow for
> userland
> > driven configuration register as ConfigFS client called
> > subsystem. Each subsystem has its own directory in ConfigFS root
> > dir. We use libcomposite which appear in ConfigFS as usb_gadget
> > directory. In this dir you can create directories called gadgets.
> Some
> > example:
> >
> > # libcomposite and configfs compiled-in
> > $ mount none -t configfs /sys/kernel/config
> > $ ls /sys/kernel/config usb_gadget
> > $ mkdir /sys/kernel/config/usb_gadget/g1
> > $ ls /sys/kernel/config/usb_gadget/g1
> > UDC              bDeviceSubClass  bcdUSB     idProduct  strings
> > bDeviceClass     bMaxPacketSize0  configs    idVendor
> > bDeviceProtocol  bcdDevice        functions  os_desc
> >
> >
> > Now let's try to use smack with ConfigFS. First of all we run to
> > similar issue as with FunctionFS so after mounting configfs with
> > smackfsdef option we still get _ label on subsystem directories
> > because they are created in configfs_register_subsystem() which
> is
> > called in libcomposite module init so really erly. So in my
> opinion
> > this looks quite similar to issue described in functionfs
> section.
> 
> Yes, "virtual" filesystems like these haven't probably ever been
> tested
> with an LSM before, as you are finding out.

Yes, I agree. I have done some more digging recently and I think that
it's conceptual problem in dcache.h API. Thanks to Hillf Danton who
asked me if I use kdbus I found out that kdbus is now also using
separate file system (kdbusfs) and has a control file just after
mount. I thought that this issue is also there but it's not.

I started to study kdbusfs implementation [1] to find out why it works
as desired. I found out that kdbusfs is not using most of dcache.h
API. Functionfs don't care about inodes for directories and just use
d_make_root() in mount and d_add() for creating a file. In kdbus
everything is done by itself. It means that it provides inode ops for
all indoes also for directories. There is no d_add() call while
creating a new file. New file is being created only as inode and then
in fs_dir_iop_lookup(), which is provided as lookup callback in
inode_operations, kdbus calls d_materialise_unique(). This means that
security label for file is initialized on first lookup in root dir
when all security flags from mount are parsed and properly stored in
super block. I think that it's why it works properly in kdbusfs.

I have no idea how to solve this. Re writing all VFS's to provide
inode operations and store only inodes instead of dentries, like
kdbusfs does, will make their code much more complicated and it's a
lot of work to be done. On the other hand I don't see an easy way how
this could be fixed in a generic way in lower layer. Maybe VFS
maintainer has some idea how to fix this?

> 
> > Second issue related to Configfs is when we create the directory.
> > Reproduction:
> >
> > $ mount none -t configfs -o smackfsdef=my_label
> /sys/kernel/config
> > $ ls -Z /sys/kernel/config
> > _ usb_gadget
> > $ chmod 777 /sys/kernel/config/usb_gadget
> > $ chsmack -a my_label /sys/kernel/config/usb_gadget
> > $ echo my_label > /proc/self/attr/current
> > $ su my_user
> > $ mkdir /sys/kernel/config/usb_gadget/g1
> > $ ls -Z /sys/kernel/config/usb_gadget/
> > _ g1
> >
> > As you see our process had my_label label but created by him
> gadget
> > has the _ label and the same with all files and subdirs created
> in
> > g1. I have no idea why this happened but definetly something is
> > wrong. Related to this is a policy of labeling subdirectories.
> Obvious
> > thing is that g1 directory should have label different that _.
> But I'm
> > not shure what label should have its children.
> >
> > 3) Generic question with multi mounts filesystems
> >
> > Some VFS (ConfigFS for example) can be mounted more than once.
> What
> > should be the correct behavior of smackfsdef mount option if we
> pass
> > different labels in each mount point?
> 
> How would it be different from a "normal" filesystem you mount at
> different locations?
> 

Normal file system will have most of labels set properly and stored in
block device as xattrs in VFS most those xattrs are stored in memory
so a lot of labels is not initialized. The question is more like: If
we can mount file system in one place as RW and in other place as RO
could we do the same with smack labels? Can we mount fs with
smackfsdef=label1 and smackfsdef=label2 in other place? How we should
understand such mount? Which label should be used? First one or last
one?

> good luck!

Thank you, I see that it will be needed;)

Footnotes:
1 - git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
branch kdbus

--
Best regards,
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
k.opasiak@samsung.com





^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: VFS and LSM issues
  2014-12-08 17:04 VFS and LSM issues Krzysztof Opasiak
  2014-12-10  4:54 ` Greg KH
@ 2014-12-10 17:18 ` Stephen Smalley
  1 sibling, 0 replies; 5+ messages in thread
From: Stephen Smalley @ 2014-12-10 17:18 UTC (permalink / raw)
  To: Krzysztof Opasiak, linux-usb, linux-kernel, linux-security-module, casey
  Cc: jlbec, 'Andrzej Pietrasiewicz',
	gregkh, 'Michal Nazarewicz', 'Robert Baldyga',
	Rafal Krypa, Tomasz Swierczek, 'Karol Lewandowski',
	'Marek Szyprowski', 'Stanislaw Wadas',
	kopasiak90, 'Łukasz Stelmach'

On 12/08/2014 12:04 PM, Krzysztof Opasiak wrote:
> Dear All,
> 
> I'm Krzysztof Opasiak from SRPOL (Samsung). I'm working on USB support
> in Tizen and mainline. In those works we use two Virtual File Systems
> - ConfigFS and FunctionFS. Recently I have tried to use them with
> SMACK and I ran into a few issues. Most of them looks to be a generic
> problem with many FS and LSM. You can find description of those issues
> and my research just below in 3 points. I'm not a VFS/LSM specialist so
> your help is very welcome;)
> 
> 1) Issues with function FS
> 
> It's a VFS which allow to provide custom USB function as userspace
> program. I know that may be quite new for you so let's define this as
> a VFS which works as follow:
> 
> $ modprobe g_ffs
> $ mkdir /tmp/mount_root
> $ mount none -t functionfs /tmp/mount_root
> $ ls /tmp/mount_root
> ep0
> 
> # now we run our program which writes some data to ep0
> #  and based on this kernel creates epX
> # you can find one in tools/usb/ffs-test.c
> 
> $ ./my_program /tmp/mount_root &
> $ ls /tmp/mount_root
> ep0 ep1 ep2
> 
> Ok so now we would like to use this together with smack. Especially
> with smackfsdef mount option. First two steps go as above and then:
> 
> $ mount none -t functionfs -o smackfsdef=my_label /tmp/mount_root
> $ ls -Z /tmp/mount_root/
>  _ ep0
> 
> Ops! Some bug here we requested to use my_label but we got _. When we
> run our program, rest of epX will get desired label (my_label). I have
> started to dig in kernel to find what happen and probably I found out
> where is a problem. Let's look to mount_fs() code which is executed
> during mount:
> 
> struct dentry *
> mount_fs(struct file_system_type *type, int flags, const char *name,
> void *data)
> {
> (...)
> 	root = type->mount(type, flags, name, data);
> (...)
> 	error = security_sb_kern_mount(sb, flags, secdata);
> (...)
> }
> 
> So what is important here is the order of operations. First is
> executed mount ops provided by selected file system. During this mount
> procedure functionfs executes new_inode(sb) to allocate inode for ep0
> which should appear directly after mount. After returning from mount
> function we execute security_sb_kern_mount() where we *parse the mount
> options* and sets the value for lsm specific structures for example we
> store the label passed in smackfsdef.
> 
> The problem here is order of calls because first we call mount for
> given fs where we create a file and after this we fill security
> structures with security mount options. While creating file in mount
> callback super block is filled only with default values for security
> so ep0 has _ label. This looks like a generic issue for all VFS which
> creates indoes before or in their mount procedure.
> 
> I'm not sure if we can simply move security_sb_kern_mount() above
> mount for specific fs, do we?

No.  Look at SELinux for an example. It puts the inode security
structures into a list and then initializes them during
security_sb_kern_mount(), both to handle labeling of inodes initialized
before first policy load and to handle inodes created during ->mount().
 In particular, look at the tail of sb_finish_set_opts() in
security/selinux/hooks.c.  As SELinux has been handling this for a long
time, I would call this a bug in Smack, not in the VFS LSM hooks.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: VFS and LSM issues
  2014-12-10  4:54 ` Greg KH
  2014-12-10 14:17   ` Krzysztof Opasiak
@ 2014-12-10 17:19   ` Stephen Smalley
  1 sibling, 0 replies; 5+ messages in thread
From: Stephen Smalley @ 2014-12-10 17:19 UTC (permalink / raw)
  To: Greg KH, Krzysztof Opasiak
  Cc: linux-usb, linux-kernel, linux-security-module, casey, jlbec,
	'Andrzej Pietrasiewicz', 'Michal Nazarewicz',
	'Robert Baldyga',
	Rafal Krypa, Tomasz Swierczek, 'Karol Lewandowski',
	'Marek Szyprowski', 'Stanislaw Wadas',
	kopasiak90, 'Łukasz Stelmach'

On 12/09/2014 11:54 PM, Greg KH wrote:
> On Mon, Dec 08, 2014 at 06:04:30PM +0100, Krzysztof Opasiak wrote:
>> Dear All,
>>
>> I'm Krzysztof Opasiak from SRPOL (Samsung). I'm working on USB support
>> in Tizen and mainline. In those works we use two Virtual File Systems
>> - ConfigFS and FunctionFS. Recently I have tried to use them with
>> SMACK and I ran into a few issues. Most of them looks to be a generic
>> problem with many FS and LSM. You can find description of those issues
>> and my research just below in 3 points. I'm not a VFS/LSM specialist so
>> your help is very welcome;)
>>
>> 1) Issues with function FS
>>
>> It's a VFS which allow to provide custom USB function as userspace
>> program. I know that may be quite new for you so let's define this as
>> a VFS which works as follow:
>>
>> $ modprobe g_ffs
>> $ mkdir /tmp/mount_root
>> $ mount none -t functionfs /tmp/mount_root
>> $ ls /tmp/mount_root
>> ep0
>>
>> # now we run our program which writes some data to ep0
>> #  and based on this kernel creates epX
>> # you can find one in tools/usb/ffs-test.c
>>
>> $ ./my_program /tmp/mount_root &
>> $ ls /tmp/mount_root
>> ep0 ep1 ep2
>>
>> Ok so now we would like to use this together with smack. Especially
>> with smackfsdef mount option. First two steps go as above and then:
>>
>> $ mount none -t functionfs -o smackfsdef=my_label /tmp/mount_root
>> $ ls -Z /tmp/mount_root/
>>  _ ep0
>>
>> Ops! Some bug here we requested to use my_label but we got _. When we
>> run our program, rest of epX will get desired label (my_label). I have
>> started to dig in kernel to find what happen and probably I found out
>> where is a problem. Let's look to mount_fs() code which is executed
>> during mount:
>>
>> struct dentry *
>> mount_fs(struct file_system_type *type, int flags, const char *name,
>> void *data)
>> {
>> (...)
>> 	root = type->mount(type, flags, name, data);
>> (...)
>> 	error = security_sb_kern_mount(sb, flags, secdata);
>> (...)
>> }
>>
>> So what is important here is the order of operations. First is
>> executed mount ops provided by selected file system. During this mount
>> procedure functionfs executes new_inode(sb) to allocate inode for ep0
>> which should appear directly after mount. After returning from mount
>> function we execute security_sb_kern_mount() where we *parse the mount
>> options* and sets the value for lsm specific structures for example we
>> store the label passed in smackfsdef.
>>
>> The problem here is order of calls because first we call mount for
>> given fs where we create a file and after this we fill security
>> structures with security mount options. While creating file in mount
>> callback super block is filled only with default values for security
>> so ep0 has _ label. This looks like a generic issue for all VFS which
>> creates indoes before or in their mount procedure.
>>
>> I'm not sure if we can simply move security_sb_kern_mount() above
>> mount for specific fs, do we?
> 
> No, I do not think you can, sorry.
> 
>> 2) Issues with ConfigFS
>>
>> ConfigFS is a generic VFS which allows to create and manage kobjects
>> from userspace. Each module which would like to allow for userland
>> driven configuration register as ConfigFS client called
>> subsystem. Each subsystem has its own directory in ConfigFS root
>> dir. We use libcomposite which appear in ConfigFS as usb_gadget
>> directory. In this dir you can create directories called gadgets. Some
>> example:
>>
>> # libcomposite and configfs compiled-in
>> $ mount none -t configfs /sys/kernel/config
>> $ ls /sys/kernel/config usb_gadget
>> $ mkdir /sys/kernel/config/usb_gadget/g1
>> $ ls /sys/kernel/config/usb_gadget/g1
>> UDC              bDeviceSubClass  bcdUSB     idProduct  strings
>> bDeviceClass     bMaxPacketSize0  configs    idVendor
>> bDeviceProtocol  bcdDevice        functions  os_desc
>>
>>
>> Now let's try to use smack with ConfigFS. First of all we run to
>> similar issue as with FunctionFS so after mounting configfs with
>> smackfsdef option we still get _ label on subsystem directories
>> because they are created in configfs_register_subsystem() which is
>> called in libcomposite module init so really erly. So in my opinion
>> this looks quite similar to issue described in functionfs section.
> 
> Yes, "virtual" filesystems like these haven't probably ever been tested
> with an LSM before, as you are finding out.

On the contrary, SELinux has been used with all of these filesystems for
quite some time...



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-12-10 17:20 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-08 17:04 VFS and LSM issues Krzysztof Opasiak
2014-12-10  4:54 ` Greg KH
2014-12-10 14:17   ` Krzysztof Opasiak
2014-12-10 17:19   ` Stephen Smalley
2014-12-10 17:18 ` Stephen Smalley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).