gmane.linux.file-systems

Re: [RFC PATCH] fs: call_usermodehelper_root helper introduced

J. Bruce Fields — 2013-05-23T20:14:31

BTW the problem I have here is that the only case I've personally had
any interest in is using network and file namespaces to isolate nfsd's
to make them safe to migrate across nodes of a cluster.

So while the idea of making user namespaces and unprivileged knfsd and
the rest work is really interesting and I'm happy to think about it, I'm
not sure how feasible or useful it is.

I'd therefore actually prefer just to take something like Stanislav's
patch now and put off the problem of how to make it work correctly with
user namespaces until we actually turn that on.  His patch fixes a real
bug that we have now, while user-namespaced-nfsd still sounds a bit
pie-in-the-sky to me.


But maybe I don't understand why Eric thinks nfsd in usernamespaces is
imminent.  Or maybe I'm missing some security problem that Stanislav's
patch would introduce now without allowing nfsd to run in a user
namespace.

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majord

NULL pointer deref after vmalloc() failure in fill_files_note()

Tommi Rantala — 2013-05-23T18:50:47

Hello,

Noticed while fuzzing with trinity, that if the vmalloc() in
fill_files_note() fails, we Oops.

I can easily reproduce the bug with this applied:

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index f8a0b0e..11b444f 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
< at >< at > -1432,7 +1432,7 < at >< at > static void fill_files_note(struct memelfnote *note)
        if (size >= MAX_FILE_NOTE_SIZE) /* paranoia check */
                goto err;
        size = round_up(size, PAGE_SIZE);
-       data = vmalloc(size);
+       data = NULL;
        if (!data)
                goto err;


Tommi

[   69.144390] BUG: unable to handle kernel NULL pointer dereference
at 0000000000000246
[   69.145015] IP: [<ffffffff814d22f0>] strim+0x80/0x80
[   69.145015] PGD b7ceb067 PUD b7df7067 PMD 0
[   69.145015] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC
[   69.145015] CPU: 0 PID: 3412 Comm: cat Not tainted 3.10.0-rc2+ #20
[   69.145015] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[   69.145015] task: ffff8800b6a00000 ti: ffff8800b7d78000 t

[PATCH 4/4] f2fs: optimize several routines in node.h

Namjae Jeon — 2013-05-23T13:58:40

From: Namjae Jeon <namjae.jeon< at >samsung.com>

There are various functions with common code which could be separated
out to make common routines. So, made new routines and in order to
retain the same call path and no major changes, written some macros
to access those routines.

Signed-off-by: Namjae Jeon <namjae.jeon< at >samsung.com>
Signed-off-by: Amit Sahrawat <a.sahrawat< at >samsung.com>
---
 fs/f2fs/node.h |   68 ++++++++++++++++----------------------------------------
 1 file changed, 19 insertions(+), 49 deletions(-)

diff --git a/fs/f2fs/node.h b/fs/f2fs/node.h
index 0a2d72f..37e1b90 100644
--- a/fs/f2fs/node.h
+++ b/fs/f2fs/node.h
< at >< at > -275,25 +275,20 < at >< at > static inline nid_t get_nid(struct page *p, int off, bool i)
  *  - Mark cold node blocks in their node footer
  *  - Mark cold data pages in page cache
  */
-static inline int is_cold_file(struct inode *inode)
-{
-return F2FS_I(inode)->i_advise & FADVISE_COLD_BIT;
-}
-
-static inline void set_cold_file(struct inode *inode)
-{
-F2FS_I(inode)->i_advise |= FADVI

Re: [RFC PATCH] fs: call_usermodehelper_root helper introduced

Jeff Layton — 2013-05-23T11:56:20

On Thu, 23 May 2013 15:38:17 +0400
Stanislav Kinsbursky <skinsbursky< at >parallels.com> wrote:


Manipulating and querying the client tracking database is an infrequent
event, so having a continuously running daemon is wasteful and means
that the admin has to ensure that it's running. A UMH upcall is much
simpler and generally "just works" if the program is present.


This program is not a daemon that runs continuously. It's only called
when the kernel needs to manipulate the database. Are you asking
whether we could turn this into a continuously running daemon? If so
then the answer is "yes", but that's not really a good idea either.

In fact, we had that with the nfsdcld program, but no one liked it
(including me) for the reasons I detailed above.

Re: [PATCH 0/3 v3] dcache: make it more scalable on large system

Dave Chinner — 2013-05-23T09:42:01

<sigh>

What was it I said about this patchset when you posted it to speed
up an Oracle benchmark back in february? I'll repeat:

"Nobody should be doing reverse dentry-to-name lookups in a quantity
sufficient for it to become a performance limiting factor."

And that makes whatever that tracepoint is doing utterly stupid.
Dumping out full paths in a tracepoint is hardly "low overhead", and
that tracepoint should be stomped on from a great height. Sure,
print the filename straight out of the dentry into a tracepoint,
but repeated calculating the full path (probably for the same set of
dentries) is just a dumb thing to do.

Anyway, your numbers show that a single AIM7 microbenchmark goes
better under tracing the specific mmap event that uses d_path(), but
the others are on average a little bit slower. That doesn't convince
me that this is worth doing. Indeed, what happens to performance
when you aren't tracing?

Indeed, have you analysed what makes that
microbenchmark contend so much on the dentry lock while

Re: [RFC PATCH] fs: call_usermodehelper_root helper introduced

Stanislav Kinsbursky — 2013-05-23T08:07:57

22.05.2013 21:33, Eric W. Biederman пишет:

Not really. Only by using a kernel module to call the UMH.
And an unprivileged can't load a module as far a I know.
I.e. NFSd, for example, will use unprivileged user's root to perform this call.


Could you, please, clarify, how setns can help here?


Hmmm...
As far as I can see (maybe I'm missing something), there main security issue that could be here
is allowing of using any passed root to swap to.
What about using the current root instead of passed one? I.e. taking the root to swap to inside the UMH.
Does this keeps the isolation on the same level?

Re: [RFC PATCH] fs: call_usermodehelper_root helper introduced

Eric W. Biederman — 2013-05-23T03:37:23


No.  By just setting the user namespace, you gain the ability to create
processes outside of your curremt rlimits, and cgroups, and pid
namespace, etc.

With the wrong set of namespaces you will talk to the wrong processes,
or utilize the wrong resources.

Outside of your current cgroup you gain the ability to use more
resources than you were limited to.

Having thought about it what I would propose is to fork a process from
the context of the mounting process (or possibly from the context of the
process that writes to the sysctl that sets the program to spawn) and
have that process be a daemon that becomes responsible for spawning user
mode helpers with context.  Either that or whe need the user mode helper
userspace infrastructure to become namespace aware and call setns.


I gave a couple and it is the classic problem with suid executables
where a lot of unexpected things are inherited from the environment, and
so things become a never ending race.  We replaced daemonize in the
kernel with just forking

[PATCH 3/3 v3] dcache: change rename_lock to a sequence read/write lock

Waiman Long — 2013-05-23T01:37:28

The d_path() and related kernel functions currently take a writer
lock on rename_lock because they need to follow pointers. By changing
rename_lock to be the new sequence read/write lock, a reader lock
can be taken and multiple d_path() threads can proceed concurrently
without blocking each other.

It is unlikely that the frequency of filesystem changes and d_path()
name lookup will be high enough to cause writer starvation, the current
limitation of the read/write lock should be acceptable in that case.

All the sites where rename_lock is referenced were modified to use the
sequence read/write lock declaration and access functions.

This patch will have merge conflict When applying to kernel version
earlier than 3.10.

Signed-off-by: Waiman Long <Waiman.Long< at >hp.com>
---
 fs/autofs4/waitq.c     |    6 ++--
 fs/ceph/mds_client.c   |    4 +-
 fs/cifs/dir.c          |    4 +-
 fs/dcache.c            |   83 ++++++++++++++++++++++++-----------------------
 fs/nfs/namespace.c     |    6 ++--
 include/linux/dcache.

Re: [PATCH 2/2] timerfd: add alarm timers

John Stultz — 2013-05-23T01:20:43

Ping? Any fsdevel folks mind taking a look at this?

Or maybe to turn this around, are there any objections to this change 
going in via tip/timers/core?

thanks
-john

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo< at >vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 10/13] NFS: Add label lifecycle management

Steve Dickson — 2013-05-22T16:50:43

From: David Quigley <dpquigl< at >davequigley.com>

From David Quigley <dpquigl< at >davequigley.com>

This patch adds the lifecycle management for the security label structure
introduced in an earlier patch. The label is not used yet but allocations and
freeing of the structure is handled.

Signed-off-by: Matthew N. Dodd <Matthew.Dodd< at >sparta.com>
Signed-off-by: Miguel Rodel Felipe <Rodel_FM< at >dsi.a-star.edu.sg>
Signed-off-by: Phua Eu Gene <PHUA_Eu_Gene< at >dsi.a-star.edu.sg>
Signed-off-by: Khin Mi Mi Aung <Mi_Mi_AUNG< at >dsi.a-star.edu.sg>
Signed-off-by: Steve Dickson <steved< at >redhat.com>
---
 fs/nfs/dir.c      | 23 ++++++++++++++++++-
 fs/nfs/inode.c    | 15 +++++++++---
 fs/nfs/nfs4proc.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++------
 3 files changed, 96 insertions(+), 11 deletions(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index e9ab2cd..736b607 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
< at >< at > -585,10 +585,16 < at >< at > int nfs_readdir_xdr_to_array(nfs_readdir_descriptor_t *desc, struct page *page,
 if (entry.fh ==

[PATCH 13/13] Kconfig: Add Kconfig entry for Labeled NFS V4 client

Steve Dickson — 2013-05-22T16:50:46

From: Steve Dickson <steved< at >redhat.com>

This patch adds the NFS_V4_SECURITY_LABEL entry which
enables security label support for the NFSv4 client

Signed-off-by: Steve Dickson <steved< at >redhat.com>
---
 fs/nfs/Kconfig | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/fs/nfs/Kconfig b/fs/nfs/Kconfig
index a048928..a63f553 100644
--- a/fs/nfs/Kconfig
+++ b/fs/nfs/Kconfig
< at >< at > -140,6 +140,25 < at >< at > config NFS_V4_1_IMPLEMENTATION_ID_DOMAIN
   If the NFS client is unchanged from the upstream kernel, this
   option should be set to the default "kernel.org".
 
+config NFS_V4_SECURITY_LABEL
+bool "Provide Security Label support for NFSv4 client"
+depends on NFS_V4_2 && SECURITY
+default y
+help
+
+Say Y here if you want enable fine-grained security label attribute
+support for NFS version 4.  Security labels allow security modules like
+SELinux and Smack to label files to facilitate enforcement of their policies.
+Without this an NFSv4 mount will have the same label on each file.
+
+If you

[PATCH 09/13] NFS:Add labels to client function prototypes

Steve Dickson — 2013-05-22T16:50:42

From: David Quigley <dpquigl< at >davequigley.com>

From David Quigley <dpquigl< at >davequigley.com>

After looking at all of the nfsv4 operations the label structure has been added
to the prototypes of the functions which can transmit label data.

Signed-off-by: Matthew N. Dodd <Matthew.Dodd< at >sparta.com>
Signed-off-by: Miguel Rodel Felipe <Rodel_FM< at >dsi.a-star.edu.sg>
Signed-off-by: Phua Eu Gene <PHUA_Eu_Gene< at >dsi.a-star.edu.sg>
Signed-off-by: Khin Mi Mi Aung <Mi_Mi_AUNG< at >dsi.a-star.edu.sg>
Signed-off-by: Steve Dickson <steved< at >redhat.com>
---
 fs/nfs/client.c         |  2 +-
 fs/nfs/dir.c            | 19 +++++-----
 fs/nfs/getroot.c        |  2 +-
 fs/nfs/inode.c          |  5 +--
 fs/nfs/namespace.c      |  2 +-
 fs/nfs/nfs3proc.c       |  7 ++--
 fs/nfs/nfs4proc.c       | 94 ++++++++++++++++++++++++++++++++-----------------
 fs/nfs/proc.c           | 13 +++----
 include/linux/nfs_fs.h  |  5 +--
 include/linux/nfs_xdr.h |  5 +--
 10 files changed, 95 insertions(+), 59 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs

[PATCH 03/13] LSM: Add flags field to security_sb_set_mnt_opts for in kernel mount data.

Steve Dickson — 2013-05-22T16:50:36

From: David Quigley <dpquigl< at >davequigley.com>

There is no way to differentiate if a text mount option is passed from user
space or the kernel. A flags field is being added to the
security_sb_set_mnt_opts hook to allow for in kernel security flags to be sent
to the LSM for processing in addition to the text options received from mount.
This patch also updated existing code to fix compilation errors.

Acked-by: Eric Paris <eparis< at >redhat.com>
Acked-by: James Morris <james.l.morris< at >oracle.com>
Signed-off-by: David P. Quigley <dpquigl< at >tycho.nsa.gov>
Signed-off-by: Miguel Rodel Felipe <Rodel_FM< at >dsi.a-star.edu.sg>
Signed-off-by: Phua Eu Gene <PHUA_Eu_Gene< at >dsi.a-star.edu.sg>
Signed-off-by: Khin Mi Mi Aung <Mi_Mi_AUNG< at >dsi.a-star.edu.sg>
---
 fs/nfs/super.c           |  3 ++-
 include/linux/security.h | 13 ++++++++++---
 security/capability.c    |  5 ++++-
 security/security.c      |  7 +++++--
 security/selinux/hooks.c | 12 ++++++++++--
 5 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/super.c

[PATCH 02/13] Security: Add Hook to test if the particular xattr is part of a MAC model.

Steve Dickson — 2013-05-22T16:50:35

From: David Quigley <dpquigl< at >davequigley.com>

The interface to request security labels from user space is the xattr
interface. When requesting the security label from an NFS server it is
important to make sure the requested xattr actually is a MAC label. This allows
us to make sure that we get the desired semantics from the attribute instead of
something else such as capabilities or a time based LSM.

Acked-by: Eric Paris <eparis< at >redhat.com>
Acked-by: James Morris <james.l.morris< at >oracle.com>
Signed-off-by: Matthew N. Dodd <Matthew.Dodd< at >sparta.com>
Signed-off-by: Miguel Rodel Felipe <Rodel_FM< at >dsi.a-star.edu.sg>
Signed-off-by: Phua Eu Gene <PHUA_Eu_Gene< at >dsi.a-star.edu.sg>
Signed-off-by: Khin Mi Mi Aung <Mi_Mi_AUNG< at >dsi.a-star.edu.sg>
---
 include/linux/security.h   | 14 ++++++++++++++
 security/capability.c      |  6 ++++++
 security/security.c        |  6 ++++++
 security/selinux/hooks.c   |  6 ++++++
 security/smack/smack_lsm.c | 11 +++++++++++
 5 files changed, 43 insertions(+)

diff --git a/include/linux/s

[RFC PATCH] fs: call_usermodehelper_root helper introduced

Stanislav Kinsbursky — 2013-05-22T07:29:18

Usermode helper executes all binaries in global "init" root context. This
doesn't allow to call a binary from other root context (for example in a
container).
Currently, both containerized NFS client and NFS server requires an ability to
execute a binary in a container's root context. Root swap can be done in
"init" callback, passed by UMH caller.
But since we have 2 callers already (and more of them are expected to appear
in future) and because set_fs_root() in not exported, it looks reasonable to
add one more generic UMH helper to generic fs code.
Root path reference must be hold by the caller, since it will be put on UMH
thread exit.

Signed-off-by: Stanislav Kinsbursky <skinsbursky< at >parallels.com>
---
 fs/fs_struct.c            |   28 ++++++++++++++++++++++++++++
 include/linux/fs_struct.h |    4 ++++
 2 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/fs/fs_struct.c b/fs/fs_struct.c
index d8ac61d..cd1de8e 100644
--- a/fs/fs_struct.c
+++ b/fs/fs_struct.c
< at >< at > -4,6 +4,7 < at >< at >
 #include <linux/path.

[PATCH v4] nilfs2: implement calculation of free inodes count

Vyacheslav Dubeyko — 2013-05-22T06:23:47

Hi Ryusuke,

This is fourth version of the patch. I reworked representation of
mi_desc_blocks_count because I think that using this field is good
solution for calculation of max entries count.

By the way, I see that it is used atomic_t type for representation
of inodes_count and blocks_count fields in nilfs_root structure.
Maybe it makes sense to rework representation of these fields too.
What do you think?

v3->v4
 * The desc_blocks value is represented by unsigned long type.
 * The nilfs_palloc_count_desc_blocks() method is simplified.
 * The mi_desc_blocks_count field of nilfs_mdt_info structure
   is represented as atomic64_t for the case of CONFIG_64BIT and
   as atomic_t for other cases.

v2->v3
 * Trivial BUG_ON checks were removed.
 * Whole calculation algorithm was moved into
   nilfs_palloc_count_max_entries() method.
 * Improve error processing in the code.
 * The nilfs_palloc_mdt_file_can_grow() method was simplified.
 * The nilfs_ifile_count_free_inodes() method was simplified.

v1->v2
 * Chang

Re: proc_subdir_lock related deadlock

Steven Rostedt — 2013-05-22T02:54:51


Do you think adding a might_sleep() to the beginning of
remove_proc_entry() would be a good idea?

Re: proc_subdir_lock related deadlock

Al Viro — 2013-05-22T02:43:51

Sigh... proc_subdir_lock is the least of your troubles; remove_proc_entry()
might block.  By design - the callers of remove_proc_entry() are free to tear
down everything needed for IO on that entry as soon as remove_proc_entry()
returns.  That's achieved by marking the entry in a way that will prevent
any new callers into ->read(), etc., then waiting for ones currently inside
any of file methods to return - once that has happened, we don't have to worry
about preserving the structures needed for those methods to work.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo< at >vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: proc_subdir_lock related deadlock

Lisa Du — 2013-05-22T01:19:13

Ok, I know, many thanks for your reply.

Thanks!

Best Regards
Lisa Du


-----Original Message-----
From: Steven Rostedt [mailto:rostedt< at >goodmis.org] 
Sent: 2013年5月22日 9:13
To: Lisa Du
Cc: linux-fsdevel< at >vger.kernel.org; Ingo Molnar
Subject: Re: proc_subdir_lock related deadlock

On Tue, 2013-05-21 at 18:06 -0700, Lisa Du wrote:

This is meaningless to me. With Linus's latest git tree:

$ git show 5a0b3548945769cf631a21e769f14a9ba8ae1c99
fatal: bad object 5a0b3548945769cf631a21e769f14a9ba8ae1c99

Re: proc_subdir_lock related deadlock

Steven Rostedt — 2013-05-22T01:13:12

This is meaningless to me. With Linus's latest git tree:

$ git show 5a0b3548945769cf631a21e769f14a9ba8ae1c99
fatal: bad object 5a0b3548945769cf631a21e769f14a9ba8ae1c99

RE: proc_subdir_lock related deadlock

Lisa Du — 2013-05-22T01:06:26

Hi, Steven
   Thanks for your reply!
   I'm using kernel3.4. The function uid_stat_tcp_rcv() was added by below patch:
   Yes, this file was removed in kernel3.8. So to avoid this happen, we should always avoid this lock used in softirq context in future right?
   And for kernel3.4, what's your opinion to do this fix?

commit 5a0b3548945769cf631a21e769f14a9ba8ae1c99
Author: Mike Chan <mike< at >android.com>
Date:   Wed Jan 7 11:40:42 2009 -0800

    misc: uidstat: Adding uid stat driver to collect network statistics.
    
    Signed-off-by: Mike Chan <mike< at >android.com>


Thanks!

Best Regards
Lisa Du


-----Original Message-----
From: Steven Rostedt [mailto:rostedt< at >goodmis.org] 
Sent: 2013年5月22日 1:39
To: Lisa Du
Cc: linux-fsdevel< at >vger.kernel.org; Ingo Molnar
Subject: Re: proc_subdir_lock related deadlock

On Sun, 2013-05-19 at 22:12 -0700, Lisa Du wrote:

I believe the real fix is not to call remove or create proc from softirq
context.

What kernel are you using, as I can't