Linux RAID subsystem development
 help / color / mirror / Atom feed
* Re: Help needed: array inactive after grow attempt
From: Andread Mayrhoff @ 2016-04-28 19:50 UTC (permalink / raw)
  To: linux-raid
In-Reply-To: <1d5a8c36a6802a9e7e32eb056165f200@spoilbase.eu>

I might add

~/tmp # mdadm --stop /dev/md127
mdadm: stopped /dev/md127
~/tmp # mdadm --assemble --scan
mdadm: Failed to restore critical section for reshape, sorry.
        Possibly you needed to specify the --backup-file

Thanks for helping me out of this...

Am 2016-04-28 21:27, schrieb Andread Mayrhoff:
> Hello,
> 
> I believe I need some help from an mdadm expert.
> 
> I tried to add a disk to an existing RAID5-array (/dev/md127) that
> consisted of 5 disks: /dev/sd[bcdef]1
> The system is a 4.5.2-2-kernel, x86_64-architecture, mdadm 3.3.1.
> I attempted to add partition /dev/sdg1 to that array, in an attempt to
> create a 6-disk-RAID5-array.
> To achieve that goal, I typed "mdadm --add /dev/md127 /dev/sdg1".
> As I found that working, I attempted to grow the array by typing
> "mdadm --grow /dev/md127 --raid-devices=6".
> 
> I left my machine, and when I returned, I found it had been switched
> off by my... ahem, anyway, it had been switched off.
> 
> I powered it on again, "cat /proc/mdstat" returned
> 
>>> 
> Personalities : [raid6] [raid5] [raid4]
> md127 : inactive sdg1[9](S) sdf1[8](S) sdc1[5](S) sdb1[6](S)
> sde1[4](S) sdd1[7](S)
>       17578345656 blocks super 1.0
> 
> unused devices: <none>
> <<
> 
> mdadm --detail /dev/md127 returns
> 
>>> 
> /dev/md127:
>         Version : 1.0
>      Raid Level : raid0
>   Total Devices : 6
>     Persistence : Superblock is persistent
> 
>           State : inactive
> 
>   Delta Devices : 1, (-1->0)
>       New Level : raid5
>      New Layout : left-symmetric
>   New Chunksize : 128K
> 
>            Name : n54l:raid5.11111111.eu
>            UUID : b120f4d9:d1ba5648:e1359c5d:7a36372e
>          Events : 60868
> 
>     Number   Major   Minor   RaidDevice
> 
>        -       8       17        -        /dev/sdb1
>        -       8       33        -        /dev/sdc1
>        -       8       49        -        /dev/sdd1
>        -       8       65        -        /dev/sde1
>        -       8       81        -        /dev/sdf1
>        -       8       97        -        /dev/sdg1
> <<
> 
> Now, before I do anything extreme like unplugging that disk again or
> forcing a recreate, could an expert suggest the next logical step I
> should do. I know this sounds pretty defensive, but rather than
> playing "Raid-hero with an erased set of disks" I'd go along with
> "Raid-idiot that needed experts' advice which is why he still's got
> all his data".
> 
> Thanks for your advice!
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" 
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply

* Help needed: array inactive after grow attempt
From: Andread Mayrhoff @ 2016-04-28 19:27 UTC (permalink / raw)
  To: linux-raid

Hello,

I believe I need some help from an mdadm expert.

I tried to add a disk to an existing RAID5-array (/dev/md127) that 
consisted of 5 disks: /dev/sd[bcdef]1
The system is a 4.5.2-2-kernel, x86_64-architecture, mdadm 3.3.1.
I attempted to add partition /dev/sdg1 to that array, in an attempt to 
create a 6-disk-RAID5-array.
To achieve that goal, I typed "mdadm --add /dev/md127 /dev/sdg1".
As I found that working, I attempted to grow the array by typing "mdadm 
--grow /dev/md127 --raid-devices=6".

I left my machine, and when I returned, I found it had been switched off 
by my... ahem, anyway, it had been switched off.

I powered it on again, "cat /proc/mdstat" returned

>> 
Personalities : [raid6] [raid5] [raid4]
md127 : inactive sdg1[9](S) sdf1[8](S) sdc1[5](S) sdb1[6](S) sde1[4](S) 
sdd1[7](S)
       17578345656 blocks super 1.0

unused devices: <none>
<<

mdadm --detail /dev/md127 returns

>> 
/dev/md127:
         Version : 1.0
      Raid Level : raid0
   Total Devices : 6
     Persistence : Superblock is persistent

           State : inactive

   Delta Devices : 1, (-1->0)
       New Level : raid5
      New Layout : left-symmetric
   New Chunksize : 128K

            Name : n54l:raid5.11111111.eu
            UUID : b120f4d9:d1ba5648:e1359c5d:7a36372e
          Events : 60868

     Number   Major   Minor   RaidDevice

        -       8       17        -        /dev/sdb1
        -       8       33        -        /dev/sdc1
        -       8       49        -        /dev/sdd1
        -       8       65        -        /dev/sde1
        -       8       81        -        /dev/sdf1
        -       8       97        -        /dev/sdg1
<<

Now, before I do anything extreme like unplugging that disk again or 
forcing a recreate, could an expert suggest the next logical step I 
should do. I know this sounds pretty defensive, but rather than playing 
"Raid-hero with an erased set of disks" I'd go along with "Raid-idiot 
that needed experts' advice which is why he still's got all his data".

Thanks for your advice!




^ permalink raw reply

* Re: [PATCH V3 08/13] md: set MD_CHANGE_PENDING in a spinlocked region
From: Shaohua Li @ 2016-04-28  3:58 UTC (permalink / raw)
  To: Guoqing Jiang; +Cc: neilb, linux-raid
In-Reply-To: <57217BAF.5060702@suse.com>

On Wed, Apr 27, 2016 at 10:55:43PM -0400, Guoqing Jiang wrote:
> 
> 
> On 04/27/2016 11:27 AM, Shaohua Li wrote:
> >On Tue, Apr 26, 2016 at 09:56:26PM -0400, Guoqing Jiang wrote:
> >>Some code waits for a metadata update by:
> >>
> >>1. flagging that it is needed (MD_CHANGE_DEVS or MD_CHANGE_CLEAN)
> >>2. setting MD_CHANGE_PENDING and waking the management thread
> >>3. waiting for MD_CHANGE_PENDING to be cleared
> >>
> >>If the first two are done without locking, the code in md_update_sb()
> >>which checks if it needs to repeat might test if an update is needed
> >>before step 1, then clear MD_CHANGE_PENDING after step 2, resulting
> >>in the wait returning early.
> >>
> >>So make sure all places that set MD_CHANGE_PENDING are protected by
> >>mddev->lock.
> >>
> >>Reviewed-by: NeilBrown <neilb@suse.com>
> >>Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> >>---
> >>V3 changes:
> >>1. use spin_lock_irqsave/spin_unlock_irqrestore in error funcs and
> >>    raid10's __make_request
> >shouldn't other places use spin_lock_irq/spin_unlock_irq? interrupt can occur
> >after you do spin_lock(), and if it's md_error, we deadlock.
> 
> It could possible in theory if func was interrupted by md_error after it
> called spin_lock,
> but seems lots of place in md.c also use spin_lock/unlock for mddev->lock,
> take
> md_do_sync and md_update_sb as example, both of them used
> spin_lock(&mddev->lock)
> and spin_unlock(&mddev->lock) before.
> 
> So I guess it will not cause trouble, otherwise, then we need to change all
> the usages of
> spin_lock/unlock(&mddev->lock), or introduce a new lock for this scenario. I
> am not sure
> which one is more acceptable.

It doesn't cause trouble, because no interrupt/bh uses lock before. But now we
use it in softirq, that's the difference. Please enable lockdep, I think it
will complain. either we change all the locking to irq save or introducing a
new lock. either is ok.

Thanks,
Shaohua

^ permalink raw reply

* Re: [PATCH 09/13] md-cluster: always setup in-memory bitmap
From: Guoqing Jiang @ 2016-04-28  2:59 UTC (permalink / raw)
  To: Shaohua Li; +Cc: neilb, linux-raid
In-Reply-To: <20160427152449.GA17061@kernel.org>




On 04/27/2016 11:24 AM, Shaohua Li wrote:
>>   static bitmap_counter_t *bitmap_get_counter(struct bitmap_counts *bitmap,
>>   					    sector_t offset, sector_t *blocks,
>> -					    int create)
>> +					    int create, int no_hijack)
>>   __releases(bitmap->lock)
>>   __acquires(bitmap->lock)
>>   {
>> @@ -1321,7 +1324,7 @@ __acquires(bitmap->lock)
>>   	sector_t csize;
>>   	int err;
>> -	err = bitmap_checkpage(bitmap, page, create);
>> +	err = bitmap_checkpage(bitmap, page, create, 0);
>>   	if (bitmap->bp[page].hijacked ||
>>   	    bitmap->bp[page].map == NULL)
>>> bitmap_get_counter doesn't use the new no_hijack parameter. And you always pass
>>> 0 to this function. so looks this change isn't required.
>>>
>> The below part of this patch pass 1 to bitmap_checkpage, so it is needed.
>>
>> +	/* For cluster raid, need to pre-allocate bitmap */
>> +	if (mddev_is_clustered(bitmap->mddev)) {
>> +		unsigned long page;
>> +		for (page = 0; page < pages; page++) {
>> +			ret = bitmap_checkpage(&bitmap->counts, page, 1, 1);
> I mean bitmap_get_counter(). You add no_hijack parameter, but not use it

Yes, I misunderstood it, thanks for point it out and will remove it.

Regards,
Guoqing

^ permalink raw reply

* Re: [PATCH V3 08/13] md: set MD_CHANGE_PENDING in a spinlocked region
From: Guoqing Jiang @ 2016-04-28  2:55 UTC (permalink / raw)
  To: Shaohua Li; +Cc: neilb, linux-raid
In-Reply-To: <20160427152753.GB17061@kernel.org>



On 04/27/2016 11:27 AM, Shaohua Li wrote:
> On Tue, Apr 26, 2016 at 09:56:26PM -0400, Guoqing Jiang wrote:
>> Some code waits for a metadata update by:
>>
>> 1. flagging that it is needed (MD_CHANGE_DEVS or MD_CHANGE_CLEAN)
>> 2. setting MD_CHANGE_PENDING and waking the management thread
>> 3. waiting for MD_CHANGE_PENDING to be cleared
>>
>> If the first two are done without locking, the code in md_update_sb()
>> which checks if it needs to repeat might test if an update is needed
>> before step 1, then clear MD_CHANGE_PENDING after step 2, resulting
>> in the wait returning early.
>>
>> So make sure all places that set MD_CHANGE_PENDING are protected by
>> mddev->lock.
>>
>> Reviewed-by: NeilBrown <neilb@suse.com>
>> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
>> ---
>> V3 changes:
>> 1. use spin_lock_irqsave/spin_unlock_irqrestore in error funcs and
>>     raid10's __make_request
> shouldn't other places use spin_lock_irq/spin_unlock_irq? interrupt can occur
> after you do spin_lock(), and if it's md_error, we deadlock.

It could possible in theory if func was interrupted by md_error after it 
called spin_lock,
but seems lots of place in md.c also use spin_lock/unlock for 
mddev->lock, take
md_do_sync and md_update_sb as example, both of them used 
spin_lock(&mddev->lock)
and spin_unlock(&mddev->lock) before.

So I guess it will not cause trouble, otherwise, then we need to change 
all the usages of
spin_lock/unlock(&mddev->lock), or introduce a new lock for this 
scenario. I am not sure
which one is more acceptable.

> please resend the whole series.

No problem, I will re-post them (except this one for now).

Thanks,
Guoqing

^ permalink raw reply

* Re: mdadm - stuck reshape operation
From: John Stoffel @ 2016-04-28  2:33 UTC (permalink / raw)
  To: Peter Bates; +Cc: linux-raid
In-Reply-To: <CAASHjeD2HqH=8ON_VSy1jbmR9NAK+NXQAmHjFw-PsW-gwNWB9g@mail.gmail.com>

>>>>> "Peter" == Peter Bates <peter.thebates@gmail.com> writes:


Peter> I have a 3 disk RAID 5 array that I tried to add a 4th disk to.

>> mdadm --add /dev/md6 /dev/sdb1
>> mdadm --grow --raid-devices=4 /dev/md6

Peter> This operation started successfully and proceeded until it hit 51.1%

>> cat /proc/mdstat
Peter> Personalities : [linear] [raid0] [raid1] [raid10] [raid6] [raid5]
Peter> [raid4] [multipath] [faulty]
Peter> md6 : active raid5 sda1[0] sdb1[5] sdf1[3] sde1[4]
Peter>       3906764800 blocks super 1.2 level 5, 512k chunk, algorithm 2 [4/4] [UUUU]
Peter>       [==========>..........]  reshape = 51.1% (998533632/1953382400)
Peter> finish=9046506.1min speed=1K/sec
Peter>       bitmap: 0/15 pages [0KB], 65536KB chunk

Peter> It has been sitting on the same 998533632 position for
Peter> days. I've tried a few reboots, but it never progresses.
Peter> Stopping the array, or trying to start the logical volume in it
Peter> hangs.  Altering the min / max speed parameters has no effect.
Peter> When I reboot and resemble the array the speed indicated
Peter> steadily drops to almost 0.

>> mdadm --assemble /dev/md6 --verbose --uuid 90c2b5c3:3bbfa0d7:a5efaeed:726c43e2

I looked back in my email archives, and I wonder if maybe you have
SElinux enabled?  If so, please turn it off and see if that helps.

What happens when you use dd on each of the drives and dump the output
to /dev/null?

Are there any messages in the logs, or dmesg output after the stuff
you showed?  Can you maybe 'strace' the mdadm process, or even go grab
the latest version using git from:

  git clone git://neil.brown.name/mdadm

And see if compiling it yourself from the master might do the trick.  


Peter> I haven't tried anything more drastic than a reboot yet,
Peter> Below is as much information as I can think to provide at this stage.
Peter> Please let me know what else I can do.
Peter> I'm happy to change kernels, kernel config or anything else require to
Peter> get better info.

Peter> Kernel: 4.4.3
Peter> mdadm 3.4

>> ps aux | grep md6
Peter> root      5041 99.9  0.0      0     0 ?        R    07:10 761:58 [md6_raid5]
Peter> root      5042  0.0  0.0      0     0 ?        D    07:10   0:00 [md6_reshape]

Peter> This is consistent. 100% cpu on the raid component, but not the reshape

>> mdadm --detail --verbose /dev/md6
Peter> /dev/md6:
Peter>         Version : 1.2
Peter>   Creation Time : Fri Aug 29 21:13:52 2014
Peter>      Raid Level : raid5
Peter>      Array Size : 3906764800 (3725.78 GiB 4000.53 GB)
Peter>   Used Dev Size : 1953382400 (1862.89 GiB 2000.26 GB)
Peter>    Raid Devices : 4
Peter>   Total Devices : 4
Peter>     Persistence : Superblock is persistent

Peter>   Intent Bitmap : Internal

Peter>     Update Time : Wed Apr 27 07:10:07 2016
Peter>           State : clean, reshaping
Peter>  Active Devices : 4
Peter> Working Devices : 4
Peter>  Failed Devices : 0
Peter>   Spare Devices : 0

Peter>          Layout : left-symmetric
Peter>      Chunk Size : 512K

Peter>  Reshape Status : 51% complete
Peter>   Delta Devices : 1, (3->4)

Peter>            Name : Alpheus:6  (local to host Alpheus)
Peter>            UUID : 90c2b5c3:3bbfa0d7:a5efaeed:726c43e2
Peter>          Events : 47975

Peter>     Number   Major   Minor   RaidDevice State
Peter>        0       8        1        0      active sync   /dev/sda1
Peter>        4       8       65        1      active sync   /dev/sde1
Peter>        3       8       81        2      active sync   /dev/sdf1
Peter>        5       8       17        3      active sync   /dev/sdb1

>> iostat
Peter> Linux 4.4.3-gentoo (Alpheus)    04/27/2016      _x86_64_        (4 CPU)

Peter> avg-cpu:  %user   %nice %system %iowait  %steal   %idle
Peter>            1.84    0.00   24.50    0.09    0.00   73.57

Peter> Looking at the individual disks I can see minor activity on the MD6
Peter> members. This activity tends to match up with the overall rate
Peter> reported by /proc/mdstat

Peter> Device:            tps    kB_read/s    kB_wrtn/s    kB_read    kB_wrtn
Peter> sda               0.02         2.72         1.69     128570      79957
Peter> sdb               0.01         0.03         1.69       1447      79889
Peter> sdd               3.85         2.27        56.08     106928    2646042
Peter> sde               0.02         2.73         1.69     128610      79961
Peter> sdf               0.02         2.72         1.69     128128      79961
Peter> sdc               4.08         5.44        56.08     256899    2646042
Peter> md0               2.91         7.62        55.08     359714    2598725
Peter> dm-0              0.00         0.03         0.00       1212          0
Peter> dm-1              0.00         0.05         0.00       2151          9
Peter> dm-2              2.65         6.52         3.42     307646     161296
Peter> dm-3              0.19         1.03        51.66      48377    2437420
Peter> md6               0.00         0.02         0.00       1036          0

>> dmesg
Peter> [ 1199.426995] md: bind<sde1>
Peter> [ 1199.427779] md: bind<sdf1>
Peter> [ 1199.428379] md: bind<sdb1>
Peter> [ 1199.428592] md: bind<sda1>
Peter> [ 1199.429260] md/raid:md6: reshape will continue
Peter> [ 1199.429274] md/raid:md6: device sda1 operational as raid disk 0
Peter> [ 1199.429275] md/raid:md6: device sdb1 operational as raid disk 3
Peter> [ 1199.429276] md/raid:md6: device sdf1 operational as raid disk 2
Peter> [ 1199.429277] md/raid:md6: device sde1 operational as raid disk 1
Peter> [ 1199.429498] md/raid:md6: allocated 4338kB
Peter> [ 1199.429807] md/raid:md6: raid level 5 active with 4 out of 4
Peter> devices, algorithm 2
Peter> [ 1199.429810] RAID conf printout:
Peter> [ 1199.429811]  --- level:5 rd:4 wd:4
Peter> [ 1199.429812]  disk 0, o:1, dev:sda1
Peter> [ 1199.429814]  disk 1, o:1, dev:sde1
Peter> [ 1199.429816]  disk 2, o:1, dev:sdf1
Peter> [ 1199.429817]  disk 3, o:1, dev:sdb1
Peter> [ 1199.429993] created bitmap (15 pages) for device md6
Peter> [ 1199.430297] md6: bitmap initialized from disk: read 1 pages, set 0
Peter> of 29807 bits
Peter> [ 1199.474604] md6: detected capacity change from 0 to 4000527155200
Peter> [ 1199.474611] md: reshape of RAID array md6
Peter> [ 1199.474613] md: minimum _guaranteed_  speed: 1000 KB/sec/disk.
Peter> [ 1199.474614] md: using maximum available idle IO bandwidth (but not
Peter> more than 200000 KB/sec) for reshape.
Peter> [ 1199.474617] md: using 128k window, over a total of 1953382400k.

>> lsblk
Peter> NAME                          MAJ:MIN RM  SIZE RO TYPE  MOUNTPOINT
Peter> sda                             8:0    0  1.8T  0 disk
Peter> └─sda1                          8:1    0  1.8T  0 part
Peter>   └─md6                         9:6    0  3.7T  0 raid5
Peter> sdb                             8:16   0  1.8T  0 disk
Peter> └─sdb1                          8:17   0  1.8T  0 part
Peter>   └─md6                         9:6    0  3.7T  0 raid5
Peter> sdc                             8:32   0  2.7T  0 disk
Peter> ├─sdc1                          8:33   0   16M  0 part
Peter> └─sdc2                          8:34   0  2.7T  0 part
Peter>   └─md0                         9:0    0  2.7T  0 raid1
Peter>     ├─vg--mirror-swap         253:0    0    4G  0 lvm   [SWAP]
Peter>     ├─vg--mirror-boot         253:1    0  256M  0 lvm   /boot
Peter>     ├─vg--mirror-root         253:2    0  256G  0 lvm   /
Peter>     └─vg--mirror-data--mirror 253:3    0  2.5T  0 lvm   /data/mirror
Peter> sdd                             8:48   0  2.7T  0 disk
Peter> ├─sdd1                          8:49   0   16M  0 part
Peter> └─sdd2                          8:50   0  2.7T  0 part
Peter>   └─md0                         9:0    0  2.7T  0 raid1
Peter>     ├─vg--mirror-swap         253:0    0    4G  0 lvm   [SWAP]
Peter>     ├─vg--mirror-boot         253:1    0  256M  0 lvm   /boot
Peter>     ├─vg--mirror-root         253:2    0  256G  0 lvm   /
Peter>     └─vg--mirror-data--mirror 253:3    0  2.5T  0 lvm   /data/mirror
Peter> sde                             8:64   0  1.8T  0 disk
Peter> └─sde1                          8:65   0  1.8T  0 part
Peter>   └─md6                         9:6    0  3.7T  0 raid5
Peter> sdf                             8:80   0  1.8T  0 disk
Peter> └─sdf1                          8:81   0  1.8T  0 part
Peter>   └─md6                         9:6    0  3.7T  0 raid5

Peter> Thanks for any pointers

Peter> Peter Bates
Peter> peter.thebates@gmail.com
Peter> --
Peter> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
Peter> the body of a message to majordomo@vger.kernel.org
Peter> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH V3 08/13] md: set MD_CHANGE_PENDING in a spinlocked region
From: Shaohua Li @ 2016-04-27 15:27 UTC (permalink / raw)
  To: Guoqing Jiang; +Cc: neilb, linux-raid
In-Reply-To: <1461722186-11023-1-git-send-email-gqjiang@suse.com>

On Tue, Apr 26, 2016 at 09:56:26PM -0400, Guoqing Jiang wrote:
> Some code waits for a metadata update by:
> 
> 1. flagging that it is needed (MD_CHANGE_DEVS or MD_CHANGE_CLEAN)
> 2. setting MD_CHANGE_PENDING and waking the management thread
> 3. waiting for MD_CHANGE_PENDING to be cleared
> 
> If the first two are done without locking, the code in md_update_sb()
> which checks if it needs to repeat might test if an update is needed
> before step 1, then clear MD_CHANGE_PENDING after step 2, resulting
> in the wait returning early.
> 
> So make sure all places that set MD_CHANGE_PENDING are protected by
> mddev->lock.
> 
> Reviewed-by: NeilBrown <neilb@suse.com>
> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> ---
> V3 changes:
> 1. use spin_lock_irqsave/spin_unlock_irqrestore in error funcs and
>    raid10's __make_request

shouldn't other places use spin_lock_irq/spin_unlock_irq? interrupt can occur
after you do spin_lock(), and if it's md_error, we deadlock.

please resend the whole series.

Thanks,
Shaohua

^ permalink raw reply

* Re: [PATCH 09/13] md-cluster: always setup in-memory bitmap
From: Shaohua Li @ 2016-04-27 15:24 UTC (permalink / raw)
  To: Guoqing Jiang; +Cc: neilb, linux-raid
In-Reply-To: <571EDEFD.3090103@suse.com>

On Tue, Apr 26, 2016 at 11:22:37AM +0800, Guoqing Jiang wrote:
> 
> 
> On 04/26/2016 01:45 AM, Shaohua Li wrote:
> >On Thu, Apr 21, 2016 at 01:58:10PM +0800, Guoqing Jiang wrote:
> >>The in-memory bitmap for raid is allocated on demand,
> >>then for cluster scenario, it is possible that slave
> >>node which received RESYNCING message doesn't have the
> >>in-memory bitmap when master node is perform resyncing,
> >>so we can't make bitmap is match up well among each
> >>nodes.
> >>
> >>So for cluster scenario, we need always preserve the
> >>bitmap, and ensure the page will not be freed. And a
> >>no_hijack flag is introduced to both bitmap_checkpage
> >>and bitmap_get_counter, which makes cluster raid returns
> >>fail once allocate failed.
> >>
> >>And the next patch is relied on this change since it
> >>keeps sync bitmap among each nodes during resyncing
> >>stage.
> >>
> >>Reviewed-by: NeilBrown <neilb@suse.com>
> >>Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> >>---
> >>  drivers/md/bitmap.c | 59 +++++++++++++++++++++++++++++++++++++++++------------
> >>  1 file changed, 46 insertions(+), 13 deletions(-)
> >>
> >>diff --git a/drivers/md/bitmap.c b/drivers/md/bitmap.c
> >>index 7df6b4f..00cf1c1 100644
> >>--- a/drivers/md/bitmap.c
> >>+++ b/drivers/md/bitmap.c
> >>@@ -46,7 +46,7 @@ static inline char *bmname(struct bitmap *bitmap)
> >>   * allocated while we're using it
> >>   */
> >>  static int bitmap_checkpage(struct bitmap_counts *bitmap,
> >>-			    unsigned long page, int create)
> >>+			    unsigned long page, int create, int no_hijack)
> >>  __releases(bitmap->lock)
> >>  __acquires(bitmap->lock)
> >>  {
> >>@@ -90,6 +90,9 @@ __acquires(bitmap->lock)
> >>  	if (mappage == NULL) {
> >>  		pr_debug("md/bitmap: map page allocation failed, hijacking\n");
> >>+		/* We don't support hijack for cluster raid */
> >>+		if (no_hijack)
> >>+			return -ENOMEM;
> >>  		/* failed - set the hijacked flag so that we can use the
> >>  		 * pointer as a counter */
> >>  		if (!bitmap->bp[page].map)
> >>@@ -1177,7 +1180,7 @@ static void bitmap_set_pending(struct bitmap_counts *bitmap, sector_t offset)
> >>  static bitmap_counter_t *bitmap_get_counter(struct bitmap_counts *bitmap,
> >>  					    sector_t offset, sector_t *blocks,
> >>-					    int create);
> >>+					    int create, int no_hijack);
> >>  /*
> >>   * bitmap daemon -- periodically wakes up to clean bits and flush pages
> >>@@ -1257,7 +1260,7 @@ void bitmap_daemon_work(struct mddev *mddev)
> >>  		}
> >>  		bmc = bitmap_get_counter(counts,
> >>  					 block,
> >>-					 &blocks, 0);
> >>+					 &blocks, 0, 0);
> >>  		if (!bmc) {
> >>  			j |= PAGE_COUNTER_MASK;
> >>@@ -1307,7 +1310,7 @@ void bitmap_daemon_work(struct mddev *mddev)
> >>  static bitmap_counter_t *bitmap_get_counter(struct bitmap_counts *bitmap,
> >>  					    sector_t offset, sector_t *blocks,
> >>-					    int create)
> >>+					    int create, int no_hijack)
> >>  __releases(bitmap->lock)
> >>  __acquires(bitmap->lock)
> >>  {
> >>@@ -1321,7 +1324,7 @@ __acquires(bitmap->lock)
> >>  	sector_t csize;
> >>  	int err;
> >>-	err = bitmap_checkpage(bitmap, page, create);
> >>+	err = bitmap_checkpage(bitmap, page, create, 0);
> >>  	if (bitmap->bp[page].hijacked ||
> >>  	    bitmap->bp[page].map == NULL)
> >bitmap_get_counter doesn't use the new no_hijack parameter. And you always pass
> >0 to this function. so looks this change isn't required.
> >
> 
> The below part of this patch pass 1 to bitmap_checkpage, so it is needed.
> 
> +	/* For cluster raid, need to pre-allocate bitmap */
> +	if (mddev_is_clustered(bitmap->mddev)) {
> +		unsigned long page;
> +		for (page = 0; page < pages; page++) {
> +			ret = bitmap_checkpage(&bitmap->counts, page, 1, 1);

I mean bitmap_get_counter(). You add no_hijack parameter, but not use it

^ permalink raw reply

* mdadm - stuck reshape operation
From: Peter Bates @ 2016-04-27 12:21 UTC (permalink / raw)
  To: linux-raid

Hi,

I have a 3 disk RAID 5 array that I tried to add a 4th disk to.

> mdadm --add /dev/md6 /dev/sdb1
> mdadm --grow --raid-devices=4 /dev/md6

This operation started successfully and proceeded until it hit 51.1%

> cat /proc/mdstat
Personalities : [linear] [raid0] [raid1] [raid10] [raid6] [raid5]
[raid4] [multipath] [faulty]
md6 : active raid5 sda1[0] sdb1[5] sdf1[3] sde1[4]
      3906764800 blocks super 1.2 level 5, 512k chunk, algorithm 2 [4/4] [UUUU]
      [==========>..........]  reshape = 51.1% (998533632/1953382400)
finish=9046506.1min speed=1K/sec
      bitmap: 0/15 pages [0KB], 65536KB chunk

It has been sitting on the same 998533632 position for days. I've
tried a few reboots, but it never progresses.
Stopping the array, or trying to start the logical volume in it hangs.
Altering the min / max speed parameters has no effect.
When I reboot and resemble the array the speed indicated steadily
drops to almost 0.

> mdadm --assemble /dev/md6 --verbose --uuid 90c2b5c3:3bbfa0d7:a5efaeed:726c43e2

I haven't tried anything more drastic than a reboot yet,
Below is as much information as I can think to provide at this stage.
Please let me know what else I can do.
I'm happy to change kernels, kernel config or anything else require to
get better info.

Kernel: 4.4.3
mdadm 3.4

> ps aux | grep md6
root      5041 99.9  0.0      0     0 ?        R    07:10 761:58 [md6_raid5]
root      5042  0.0  0.0      0     0 ?        D    07:10   0:00 [md6_reshape]

This is consistent. 100% cpu on the raid component, but not the reshape

> mdadm --detail --verbose /dev/md6
/dev/md6:
        Version : 1.2
  Creation Time : Fri Aug 29 21:13:52 2014
     Raid Level : raid5
     Array Size : 3906764800 (3725.78 GiB 4000.53 GB)
  Used Dev Size : 1953382400 (1862.89 GiB 2000.26 GB)
   Raid Devices : 4
  Total Devices : 4
    Persistence : Superblock is persistent

  Intent Bitmap : Internal

    Update Time : Wed Apr 27 07:10:07 2016
          State : clean, reshaping
 Active Devices : 4
Working Devices : 4
 Failed Devices : 0
  Spare Devices : 0

         Layout : left-symmetric
     Chunk Size : 512K

 Reshape Status : 51% complete
  Delta Devices : 1, (3->4)

           Name : Alpheus:6  (local to host Alpheus)
           UUID : 90c2b5c3:3bbfa0d7:a5efaeed:726c43e2
         Events : 47975

    Number   Major   Minor   RaidDevice State
       0       8        1        0      active sync   /dev/sda1
       4       8       65        1      active sync   /dev/sde1
       3       8       81        2      active sync   /dev/sdf1
       5       8       17        3      active sync   /dev/sdb1

> iostat
Linux 4.4.3-gentoo (Alpheus)    04/27/2016      _x86_64_        (4 CPU)

avg-cpu:  %user   %nice %system %iowait  %steal   %idle
           1.84    0.00   24.50    0.09    0.00   73.57

Looking at the individual disks I can see minor activity on the MD6
members. This activity tends to match up with the overall rate
reported by /proc/mdstat

Device:            tps    kB_read/s    kB_wrtn/s    kB_read    kB_wrtn
sda               0.02         2.72         1.69     128570      79957
sdb               0.01         0.03         1.69       1447      79889
sdd               3.85         2.27        56.08     106928    2646042
sde               0.02         2.73         1.69     128610      79961
sdf               0.02         2.72         1.69     128128      79961
sdc               4.08         5.44        56.08     256899    2646042
md0               2.91         7.62        55.08     359714    2598725
dm-0              0.00         0.03         0.00       1212          0
dm-1              0.00         0.05         0.00       2151          9
dm-2              2.65         6.52         3.42     307646     161296
dm-3              0.19         1.03        51.66      48377    2437420
md6               0.00         0.02         0.00       1036          0

> dmesg
[ 1199.426995] md: bind<sde1>
[ 1199.427779] md: bind<sdf1>
[ 1199.428379] md: bind<sdb1>
[ 1199.428592] md: bind<sda1>
[ 1199.429260] md/raid:md6: reshape will continue
[ 1199.429274] md/raid:md6: device sda1 operational as raid disk 0
[ 1199.429275] md/raid:md6: device sdb1 operational as raid disk 3
[ 1199.429276] md/raid:md6: device sdf1 operational as raid disk 2
[ 1199.429277] md/raid:md6: device sde1 operational as raid disk 1
[ 1199.429498] md/raid:md6: allocated 4338kB
[ 1199.429807] md/raid:md6: raid level 5 active with 4 out of 4
devices, algorithm 2
[ 1199.429810] RAID conf printout:
[ 1199.429811]  --- level:5 rd:4 wd:4
[ 1199.429812]  disk 0, o:1, dev:sda1
[ 1199.429814]  disk 1, o:1, dev:sde1
[ 1199.429816]  disk 2, o:1, dev:sdf1
[ 1199.429817]  disk 3, o:1, dev:sdb1
[ 1199.429993] created bitmap (15 pages) for device md6
[ 1199.430297] md6: bitmap initialized from disk: read 1 pages, set 0
of 29807 bits
[ 1199.474604] md6: detected capacity change from 0 to 4000527155200
[ 1199.474611] md: reshape of RAID array md6
[ 1199.474613] md: minimum _guaranteed_  speed: 1000 KB/sec/disk.
[ 1199.474614] md: using maximum available idle IO bandwidth (but not
more than 200000 KB/sec) for reshape.
[ 1199.474617] md: using 128k window, over a total of 1953382400k.

> lsblk
NAME                          MAJ:MIN RM  SIZE RO TYPE  MOUNTPOINT
sda                             8:0    0  1.8T  0 disk
└─sda1                          8:1    0  1.8T  0 part
  └─md6                         9:6    0  3.7T  0 raid5
sdb                             8:16   0  1.8T  0 disk
└─sdb1                          8:17   0  1.8T  0 part
  └─md6                         9:6    0  3.7T  0 raid5
sdc                             8:32   0  2.7T  0 disk
├─sdc1                          8:33   0   16M  0 part
└─sdc2                          8:34   0  2.7T  0 part
  └─md0                         9:0    0  2.7T  0 raid1
    ├─vg--mirror-swap         253:0    0    4G  0 lvm   [SWAP]
    ├─vg--mirror-boot         253:1    0  256M  0 lvm   /boot
    ├─vg--mirror-root         253:2    0  256G  0 lvm   /
    └─vg--mirror-data--mirror 253:3    0  2.5T  0 lvm   /data/mirror
sdd                             8:48   0  2.7T  0 disk
├─sdd1                          8:49   0   16M  0 part
└─sdd2                          8:50   0  2.7T  0 part
  └─md0                         9:0    0  2.7T  0 raid1
    ├─vg--mirror-swap         253:0    0    4G  0 lvm   [SWAP]
    ├─vg--mirror-boot         253:1    0  256M  0 lvm   /boot
    ├─vg--mirror-root         253:2    0  256G  0 lvm   /
    └─vg--mirror-data--mirror 253:3    0  2.5T  0 lvm   /data/mirror
sde                             8:64   0  1.8T  0 disk
└─sde1                          8:65   0  1.8T  0 part
  └─md6                         9:6    0  3.7T  0 raid5
sdf                             8:80   0  1.8T  0 disk
└─sdf1                          8:81   0  1.8T  0 part
  └─md6                         9:6    0  3.7T  0 raid5

Thanks for any pointers

Peter Bates
peter.thebates@gmail.com
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v4 17/21] capabilities: Allow privileged user in s_user_ns to set security.* xattrs
From: James Morris @ 2016-04-27  7:22 UTC (permalink / raw)
  To: Seth Forshee
  Cc: Eric W. Biederman, Serge Hallyn, James Morris, Serge E. Hallyn,
	Alexander Viro, Richard Weinberger, Austin S Hemmelgarn,
	Miklos Szeredi, Pavel Tikhomirov,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-bcache-u79uwXL29TY76Z2rM5mHXA,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	selinux-+05T5uksL2qpZYMLLGbcSA, cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1461699396-33000-18-git-send-email-seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

On Tue, 26 Apr 2016, Seth Forshee wrote:

> A privileged user in s_user_ns will generally have the ability to
> manipulate the backing store and insert security.* xattrs into
> the filesystem directly. Therefore the kernel must be prepared to
> handle these xattrs from unprivileged mounts, and it makes little
> sense for commoncap to prevent writing these xattrs to the
> filesystem. The capability and LSM code have already been updated
> to appropriately handle xattrs from unprivileged mounts, so it
> is safe to loosen this restriction on setting xattrs.
> 
> The exception to this logic is that writing xattrs to a mounted
> filesystem may also cause the LSM inode_post_setxattr or
> inode_setsecurity callbacks to be invoked. SELinux will deny the
> xattr update by virtue of applying mountpoint labeling to
> unprivileged userns mounts, and Smack will deny the writes for
> any user without global CAP_MAC_ADMIN, so loosening the
> capability check in commoncap is safe in this respect as well.
> 
> Signed-off-by: Seth Forshee <seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> Acked-by: Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>


Acked-by: James Morris <james.l.morris-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>


-- 
James Morris
<jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org>

^ permalink raw reply

* [PATCH V3 08/13] md: set MD_CHANGE_PENDING in a spinlocked region
From: Guoqing Jiang @ 2016-04-27  1:56 UTC (permalink / raw)
  To: shli; +Cc: neilb, linux-raid, Guoqing Jiang
In-Reply-To: <1461221895-20688-1-git-send-email-gqjiang@suse.com>

Some code waits for a metadata update by:

1. flagging that it is needed (MD_CHANGE_DEVS or MD_CHANGE_CLEAN)
2. setting MD_CHANGE_PENDING and waking the management thread
3. waiting for MD_CHANGE_PENDING to be cleared

If the first two are done without locking, the code in md_update_sb()
which checks if it needs to repeat might test if an update is needed
before step 1, then clear MD_CHANGE_PENDING after step 2, resulting
in the wait returning early.

So make sure all places that set MD_CHANGE_PENDING are protected by
mddev->lock.

Reviewed-by: NeilBrown <neilb@suse.com>
Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
---
V3 changes:
1. use spin_lock_irqsave/spin_unlock_irqrestore in error funcs and
   raid10's __make_request

V2 Changes:
1. s/write_lock/lock which is reported by auto build

 drivers/md/md.c          | 22 +++++++++++++++++-----
 drivers/md/raid1.c       |  2 ++
 drivers/md/raid10.c      |  6 +++++-
 drivers/md/raid5-cache.c |  2 ++
 drivers/md/raid5.c       |  2 ++
 5 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 06f6e81..943c953 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -2295,12 +2295,18 @@ repeat:
 	if (mddev_is_clustered(mddev)) {
 		if (test_and_clear_bit(MD_CHANGE_DEVS, &mddev->flags))
 			force_change = 1;
+		if (test_and_clear_bit(MD_CHANGE_CLEAN, &mddev->flags))
+			nospares = 1;
 		ret = md_cluster_ops->metadata_update_start(mddev);
 		/* Has someone else has updated the sb */
 		if (!does_sb_need_changing(mddev)) {
 			if (ret == 0)
 				md_cluster_ops->metadata_update_cancel(mddev);
-			clear_bit(MD_CHANGE_PENDING, &mddev->flags);
+			spin_lock(&mddev->lock);
+			if (!test_bit(MD_CHANGE_DEVS, &mddev->flags) &&
+			    !test_bit(MD_CHANGE_CLEAN, &mddev->flags))
+				clear_bit(MD_CHANGE_PENDING, &mddev->flags);
+			spin_unlock(&mddev->lock);
 			return;
 		}
 	}
@@ -2436,7 +2442,8 @@ repeat:
 
 	spin_lock(&mddev->lock);
 	if (mddev->in_sync != sync_req ||
-	    test_bit(MD_CHANGE_DEVS, &mddev->flags)) {
+	    test_bit(MD_CHANGE_DEVS, &mddev->flags) ||
+	    test_bit(MD_CHANGE_CLEAN, &mddev->flags)) {
 		/* have to write it out again */
 		spin_unlock(&mddev->lock);
 		goto repeat;
@@ -8147,18 +8154,20 @@ void md_do_sync(struct md_thread *thread)
 		}
 	}
  skip:
-	set_bit(MD_CHANGE_DEVS, &mddev->flags);
-
 	if (mddev_is_clustered(mddev) &&
 	    ret == 0) {
 		/* set CHANGE_PENDING here since maybe another
 		 * update is needed, so other nodes are informed */
+		spin_lock(&mddev->lock);
+		set_bit(MD_CHANGE_DEVS, &mddev->flags);
 		set_bit(MD_CHANGE_PENDING, &mddev->flags);
+		spin_unlock(&mddev->lock);
 		md_wakeup_thread(mddev->thread);
 		wait_event(mddev->sb_wait,
 			   !test_bit(MD_CHANGE_PENDING, &mddev->flags));
 		md_cluster_ops->resync_finish(mddev);
-	}
+	} else
+		set_bit(MD_CHANGE_DEVS, &mddev->flags);
 
 	spin_lock(&mddev->lock);
 	if (!test_bit(MD_RECOVERY_INTR, &mddev->recovery)) {
@@ -8550,6 +8559,7 @@ EXPORT_SYMBOL(md_finish_reshape);
 int rdev_set_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
 		       int is_new)
 {
+	struct mddev *mddev = rdev->mddev;
 	int rv;
 	if (is_new)
 		s += rdev->new_data_offset;
@@ -8559,8 +8569,10 @@ int rdev_set_badblocks(struct md_rdev *rdev, sector_t s, int sectors,
 	if (rv == 0) {
 		/* Make sure they get written out promptly */
 		sysfs_notify_dirent_safe(rdev->sysfs_state);
+		spin_lock(&mddev->lock);
 		set_bit(MD_CHANGE_CLEAN, &rdev->mddev->flags);
 		set_bit(MD_CHANGE_PENDING, &rdev->mddev->flags);
+		spin_unlock(&mddev->lock);
 		md_wakeup_thread(rdev->mddev->thread);
 		return 1;
 	} else
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index a7f2b9c..cffd224 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1474,8 +1474,10 @@ static void raid1_error(struct mddev *mddev, struct md_rdev *rdev)
 	 * if recovery is running, make sure it aborts.
 	 */
 	set_bit(MD_RECOVERY_INTR, &mddev->recovery);
+	spin_lock_irqsave(&mddev->lock, flags);
 	set_bit(MD_CHANGE_DEVS, &mddev->flags);
 	set_bit(MD_CHANGE_PENDING, &mddev->flags);
+	spin_unlock_irqrestore(&mddev->lock, flags);
 	printk(KERN_ALERT
 	       "md/raid1:%s: Disk failure on %s, disabling device.\n"
 	       "md/raid1:%s: Operation continuing on %d devices.\n",
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index e3fd725..cd615fe 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1102,8 +1102,10 @@ static void __make_request(struct mddev *mddev, struct bio *bio)
 		bio->bi_iter.bi_sector < conf->reshape_progress))) {
 		/* Need to update reshape_position in metadata */
 		mddev->reshape_position = conf->reshape_progress;
+		spin_lock_irqsave(&mddev->lock, flags);
 		set_bit(MD_CHANGE_DEVS, &mddev->flags);
 		set_bit(MD_CHANGE_PENDING, &mddev->flags);
+		spin_unlock_irqrestore(&mddev->lock, flags);
 		md_wakeup_thread(mddev->thread);
 		wait_event(mddev->sb_wait,
 			   !test_bit(MD_CHANGE_PENDING, &mddev->flags));
@@ -1585,15 +1587,17 @@ static void raid10_error(struct mddev *mddev, struct md_rdev *rdev)
 	}
 	if (test_and_clear_bit(In_sync, &rdev->flags))
 		mddev->degraded++;
+	spin_unlock_irqrestore(&conf->device_lock, flags);
 	/*
 	 * If recovery is running, make sure it aborts.
 	 */
 	set_bit(MD_RECOVERY_INTR, &mddev->recovery);
 	set_bit(Blocked, &rdev->flags);
 	set_bit(Faulty, &rdev->flags);
+	spin_lock_irqsave(&mddev->lock, flags);
 	set_bit(MD_CHANGE_DEVS, &mddev->flags);
 	set_bit(MD_CHANGE_PENDING, &mddev->flags);
-	spin_unlock_irqrestore(&conf->device_lock, flags);
+	spin_unlock_irqrestore(&mddev->lock, flags);
 	printk(KERN_ALERT
 	       "md/raid10:%s: Disk failure on %s, disabling device.\n"
 	       "md/raid10:%s: Operation continuing on %d devices.\n",
diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 9531f5f..2ba9366 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -712,8 +712,10 @@ static void r5l_write_super_and_discard_space(struct r5l_log *log,
 	 * in_teardown check workaround this issue.
 	 */
 	if (!log->in_teardown) {
+		spin_lock(&mddev->lock);
 		set_bit(MD_CHANGE_DEVS, &mddev->flags);
 		set_bit(MD_CHANGE_PENDING, &mddev->flags);
+		spin_unlock(&mddev->lock);
 		md_wakeup_thread(mddev->thread);
 		wait_event(mddev->sb_wait,
 			!test_bit(MD_CHANGE_PENDING, &mddev->flags) ||
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 8ab8b65..fffa53f 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -2514,8 +2514,10 @@ static void raid5_error(struct mddev *mddev, struct md_rdev *rdev)
 
 	set_bit(Blocked, &rdev->flags);
 	set_bit(Faulty, &rdev->flags);
+	spin_lock_irqsave(&mddev->lock, flags);
 	set_bit(MD_CHANGE_DEVS, &mddev->flags);
 	set_bit(MD_CHANGE_PENDING, &mddev->flags);
+	spin_unlock_irqrestore(&mddev->lock, flags);
 	printk(KERN_ALERT
 	       "md/raid:%s: Disk failure on %s, disabling device.\n"
 	       "md/raid:%s: Operation continuing on %d devices.\n",
-- 
2.6.6


^ permalink raw reply related

* [PATCH v4 21/21] fuse: Allow user namespace mounts
From: Seth Forshee @ 2016-04-26 19:36 UTC (permalink / raw)
  To: Eric W. Biederman, Miklos Szeredi
  Cc: Alexander Viro, Serge Hallyn, Richard Weinberger,
	Austin S Hemmelgarn, Miklos Szeredi, Pavel Tikhomirov,
	linux-kernel, linux-bcache, dm-devel, linux-raid, linux-mtd,
	linux-fsdevel, fuse-devel, linux-security-module, selinux,
	cgroups, Seth Forshee
In-Reply-To: <1461699396-33000-1-git-send-email-seth.forshee@canonical.com>

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
Acked-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/fuse/inode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 0a771145d853..254f1944ee98 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1199,7 +1199,7 @@ static void fuse_kill_sb_anon(struct super_block *sb)
 static struct file_system_type fuse_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "fuse",
-	.fs_flags	= FS_HAS_SUBTYPE,
+	.fs_flags	= FS_HAS_SUBTYPE | FS_USERNS_MOUNT,
 	.mount		= fuse_mount,
 	.kill_sb	= fuse_kill_sb_anon,
 };
@@ -1231,7 +1231,7 @@ static struct file_system_type fuseblk_fs_type = {
 	.name		= "fuseblk",
 	.mount		= fuse_mount_blk,
 	.kill_sb	= fuse_kill_sb_blk,
-	.fs_flags	= FS_REQUIRES_DEV | FS_HAS_SUBTYPE,
+	.fs_flags	= FS_REQUIRES_DEV | FS_HAS_SUBTYPE | FS_USERNS_MOUNT,
 };
 MODULE_ALIAS_FS("fuseblk");
 
-- 
2.7.4


^ permalink raw reply related

* [PATCH v4 20/21] fuse: Restrict allow_other to the superblock's namespace or a descendant
From: Seth Forshee @ 2016-04-26 19:36 UTC (permalink / raw)
  To: Eric W. Biederman, Miklos Szeredi
  Cc: Alexander Viro, Serge Hallyn, Richard Weinberger,
	Austin S Hemmelgarn, Miklos Szeredi, Pavel Tikhomirov,
	linux-kernel, linux-bcache, dm-devel, linux-raid, linux-mtd,
	linux-fsdevel, fuse-devel, linux-security-module, selinux,
	cgroups, Seth Forshee
In-Reply-To: <1461699396-33000-1-git-send-email-seth.forshee@canonical.com>

Unprivileged users are normally restricted from mounting with the
allow_other option by system policy, but this could be bypassed
for a mount done with user namespace root permissions. In such
cases allow_other should not allow users outside the userns
to access the mount as doing so would give the unprivileged user
the ability to manipulate processes it would otherwise be unable
to manipulate. Restrict allow_other to apply to users in the same
userns used at mount or a descendant of that namespace. Also
export current_in_userns() for use by fuse when built as a
module.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
Acked-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/fuse/dir.c           | 2 +-
 kernel/user_namespace.c | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index ecba75bf6640..1a6c5af49608 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1015,7 +1015,7 @@ int fuse_allow_current_process(struct fuse_conn *fc)
 	const struct cred *cred;
 
 	if (fc->flags & FUSE_ALLOW_OTHER)
-		return 1;
+		return current_in_userns(fc->user_ns);
 
 	cred = current_cred();
 	if (uid_eq(cred->euid, fc->user_id) &&
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 68f594212759..fa2294e14b77 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -951,6 +951,7 @@ bool current_in_userns(const struct user_namespace *target_ns)
 	}
 	return false;
 }
+EXPORT_SYMBOL(current_in_userns);
 
 static inline struct user_namespace *to_user_ns(struct ns_common *ns)
 {
-- 
2.7.4


^ permalink raw reply related

* [PATCH v4 19/21] fuse: Support fuse filesystems outside of init_user_ns
From: Seth Forshee @ 2016-04-26 19:36 UTC (permalink / raw)
  To: Eric W. Biederman, Miklos Szeredi
  Cc: linux-bcache-u79uwXL29TY76Z2rM5mHXA, Serge Hallyn, Seth Forshee,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA, Miklos Szeredi,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Austin S Hemmelgarn,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Alexander Viro,
	selinux-+05T5uksL2qpZYMLLGbcSA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	cgroups-u79uwXL29TY76Z2rM5mHXA, Pavel Tikhomirov
In-Reply-To: <1461699396-33000-1-git-send-email-seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

In order to support mounts from namespaces other than
init_user_ns, fuse must translate uids and gids to/from the
userns of the process servicing requests on /dev/fuse. This
patch does that, with a couple of restrictions on the namespace:

 - The userns for the fuse connection is fixed to the namespace
   from which /dev/fuse is opened.

 - The namespace must be the same as s_user_ns.

These restrictions simplify the implementation by avoiding the
need to pass around userns references and by allowing fuse to
rely on the checks in inode_change_ok for ownership changes.
Either restriction could be relaxed in the future if needed.

For cuse the namespace used for the connection is also simply
current_user_ns() at the time /dev/cuse is opened.

Signed-off-by: Seth Forshee <seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
 fs/fuse/cuse.c   |  3 ++-
 fs/fuse/dev.c    | 13 ++++++++-----
 fs/fuse/dir.c    | 14 +++++++-------
 fs/fuse/fuse_i.h |  6 +++++-
 fs/fuse/inode.c  | 33 +++++++++++++++++++++------------
 5 files changed, 43 insertions(+), 26 deletions(-)

diff --git a/fs/fuse/cuse.c b/fs/fuse/cuse.c
index c5b6b7165489..98ebd0f4fd4c 100644
--- a/fs/fuse/cuse.c
+++ b/fs/fuse/cuse.c
@@ -48,6 +48,7 @@
 #include <linux/stat.h>
 #include <linux/module.h>
 #include <linux/uio.h>
+#include <linux/user_namespace.h>
 
 #include "fuse_i.h"
 
@@ -498,7 +499,7 @@ static int cuse_channel_open(struct inode *inode, struct file *file)
 	if (!cc)
 		return -ENOMEM;
 
-	fuse_conn_init(&cc->fc);
+	fuse_conn_init(&cc->fc, current_user_ns());
 
 	fud = fuse_dev_alloc(&cc->fc);
 	if (!fud) {
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 4e91b2ac25a7..8fa1ce934df3 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -127,8 +127,8 @@ static void __fuse_put_request(struct fuse_req *req)
 
 static void fuse_req_init_context(struct fuse_conn *fc, struct fuse_req *req)
 {
-	req->in.h.uid = from_kuid_munged(&init_user_ns, current_fsuid());
-	req->in.h.gid = from_kgid_munged(&init_user_ns, current_fsgid());
+	req->in.h.uid = from_kuid(fc->user_ns, current_fsuid());
+	req->in.h.gid = from_kgid(fc->user_ns, current_fsgid());
 	req->in.h.pid = pid_nr_ns(task_pid(current), fc->pid_ns);
 }
 
@@ -186,7 +186,8 @@ static struct fuse_req *__fuse_get_req(struct fuse_conn *fc, unsigned npages,
 	__set_bit(FR_WAITING, &req->flags);
 	if (for_background)
 		__set_bit(FR_BACKGROUND, &req->flags);
-	if (req->in.h.pid == 0) {
+	if (req->in.h.pid == 0 || req->in.h.uid == (uid_t)-1 ||
+	    req->in.h.gid == (gid_t)-1) {
 		fuse_put_request(fc, req);
 		return ERR_PTR(-EOVERFLOW);
 	}
@@ -1248,7 +1249,8 @@ static ssize_t fuse_dev_do_read(struct fuse_dev *fud, struct file *file,
 	struct fuse_in *in;
 	unsigned reqsize;
 
-	if (task_active_pid_ns(current) != fc->pid_ns)
+	if (task_active_pid_ns(current) != fc->pid_ns ||
+	    current_user_ns() != fc->user_ns)
 		return -EIO;
 
  restart:
@@ -1880,7 +1882,8 @@ static ssize_t fuse_dev_do_write(struct fuse_dev *fud,
 	struct fuse_req *req;
 	struct fuse_out_header oh;
 
-	if (task_active_pid_ns(current) != fc->pid_ns)
+	if (task_active_pid_ns(current) != fc->pid_ns ||
+	    current_user_ns() != fc->user_ns)
 		return -EIO;
 
 	if (nbytes < sizeof(struct fuse_out_header))
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 4b855b65d457..ecba75bf6640 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -841,8 +841,8 @@ static void fuse_fillattr(struct inode *inode, struct fuse_attr *attr,
 	stat->ino = attr->ino;
 	stat->mode = (inode->i_mode & S_IFMT) | (attr->mode & 07777);
 	stat->nlink = attr->nlink;
-	stat->uid = make_kuid(&init_user_ns, attr->uid);
-	stat->gid = make_kgid(&init_user_ns, attr->gid);
+	stat->uid = make_kuid(fc->user_ns, attr->uid);
+	stat->gid = make_kgid(fc->user_ns, attr->gid);
 	stat->rdev = inode->i_rdev;
 	stat->atime.tv_sec = attr->atime;
 	stat->atime.tv_nsec = attr->atimensec;
@@ -1459,17 +1459,17 @@ static bool update_mtime(unsigned ivalid, bool trust_local_mtime)
 	return true;
 }
 
-static void iattr_to_fattr(struct iattr *iattr, struct fuse_setattr_in *arg,
-			   bool trust_local_cmtime)
+static void iattr_to_fattr(struct fuse_conn *fc, struct iattr *iattr,
+			   struct fuse_setattr_in *arg, bool trust_local_cmtime)
 {
 	unsigned ivalid = iattr->ia_valid;
 
 	if (ivalid & ATTR_MODE)
 		arg->valid |= FATTR_MODE,   arg->mode = iattr->ia_mode;
 	if (ivalid & ATTR_UID)
-		arg->valid |= FATTR_UID,    arg->uid = from_kuid(&init_user_ns, iattr->ia_uid);
+		arg->valid |= FATTR_UID,    arg->uid = from_kuid(fc->user_ns, iattr->ia_uid);
 	if (ivalid & ATTR_GID)
-		arg->valid |= FATTR_GID,    arg->gid = from_kgid(&init_user_ns, iattr->ia_gid);
+		arg->valid |= FATTR_GID,    arg->gid = from_kgid(fc->user_ns, iattr->ia_gid);
 	if (ivalid & ATTR_SIZE)
 		arg->valid |= FATTR_SIZE,   arg->size = iattr->ia_size;
 	if (ivalid & ATTR_ATIME) {
@@ -1629,7 +1629,7 @@ int fuse_do_setattr(struct inode *inode, struct iattr *attr,
 
 	memset(&inarg, 0, sizeof(inarg));
 	memset(&outarg, 0, sizeof(outarg));
-	iattr_to_fattr(attr, &inarg, trust_local_cmtime);
+	iattr_to_fattr(fc, attr, &inarg, trust_local_cmtime);
 	if (file) {
 		struct fuse_file *ff = file->private_data;
 		inarg.valid |= FATTR_FH;
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 9145445a759a..9f4c3c82edd6 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -24,6 +24,7 @@
 #include <linux/workqueue.h>
 #include <linux/kref.h>
 #include <linux/pid_namespace.h>
+#include <linux/user_namespace.h>
 
 /** Max number of pages that can be used in a single read request */
 #define FUSE_MAX_PAGES_PER_REQ 32
@@ -469,6 +470,9 @@ struct fuse_conn {
 	/** The pid namespace for this mount */
 	struct pid_namespace *pid_ns;
 
+	/** The user namespace for this mount */
+	struct user_namespace *user_ns;
+
 	/** The fuse mount flags for this mount */
 	unsigned flags;
 
@@ -867,7 +871,7 @@ struct fuse_conn *fuse_conn_get(struct fuse_conn *fc);
 /**
  * Initialize fuse_conn
  */
-void fuse_conn_init(struct fuse_conn *fc);
+void fuse_conn_init(struct fuse_conn *fc, struct user_namespace *user_ns);
 
 /**
  * Release reference to fuse_conn
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index eade0bfa4488..0a771145d853 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -167,8 +167,8 @@ void fuse_change_attributes_common(struct inode *inode, struct fuse_attr *attr,
 	inode->i_ino     = fuse_squash_ino(attr->ino);
 	inode->i_mode    = (inode->i_mode & S_IFMT) | (attr->mode & 07777);
 	set_nlink(inode, attr->nlink);
-	inode->i_uid     = make_kuid(&init_user_ns, attr->uid);
-	inode->i_gid     = make_kgid(&init_user_ns, attr->gid);
+	inode->i_uid     = make_kuid(fc->user_ns, attr->uid);
+	inode->i_gid     = make_kgid(fc->user_ns, attr->gid);
 	inode->i_blocks  = attr->blocks;
 	inode->i_atime.tv_sec   = attr->atime;
 	inode->i_atime.tv_nsec  = attr->atimensec;
@@ -467,7 +467,8 @@ static int fuse_match_uint(substring_t *s, unsigned int *res)
 	return err;
 }
 
-static int parse_fuse_opt(char *opt, struct fuse_mount_data *d, int is_bdev)
+static int parse_fuse_opt(char *opt, struct fuse_mount_data *d, int is_bdev,
+			  struct user_namespace *user_ns)
 {
 	char *p;
 	memset(d, 0, sizeof(struct fuse_mount_data));
@@ -503,7 +504,7 @@ static int parse_fuse_opt(char *opt, struct fuse_mount_data *d, int is_bdev)
 		case OPT_USER_ID:
 			if (fuse_match_uint(&args[0], &uv))
 				return 0;
-			d->user_id = make_kuid(current_user_ns(), uv);
+			d->user_id = make_kuid(user_ns, uv);
 			if (!uid_valid(d->user_id))
 				return 0;
 			d->user_id_present = 1;
@@ -512,7 +513,7 @@ static int parse_fuse_opt(char *opt, struct fuse_mount_data *d, int is_bdev)
 		case OPT_GROUP_ID:
 			if (fuse_match_uint(&args[0], &uv))
 				return 0;
-			d->group_id = make_kgid(current_user_ns(), uv);
+			d->group_id = make_kgid(user_ns, uv);
 			if (!gid_valid(d->group_id))
 				return 0;
 			d->group_id_present = 1;
@@ -555,8 +556,10 @@ static int fuse_show_options(struct seq_file *m, struct dentry *root)
 	struct super_block *sb = root->d_sb;
 	struct fuse_conn *fc = get_fuse_conn_super(sb);
 
-	seq_printf(m, ",user_id=%u", from_kuid_munged(&init_user_ns, fc->user_id));
-	seq_printf(m, ",group_id=%u", from_kgid_munged(&init_user_ns, fc->group_id));
+	seq_printf(m, ",user_id=%u",
+		   from_kuid_munged(fc->user_ns, fc->user_id));
+	seq_printf(m, ",group_id=%u",
+		   from_kgid_munged(fc->user_ns, fc->group_id));
 	if (fc->flags & FUSE_DEFAULT_PERMISSIONS)
 		seq_puts(m, ",default_permissions");
 	if (fc->flags & FUSE_ALLOW_OTHER)
@@ -587,7 +590,7 @@ static void fuse_pqueue_init(struct fuse_pqueue *fpq)
 	fpq->connected = 1;
 }
 
-void fuse_conn_init(struct fuse_conn *fc)
+void fuse_conn_init(struct fuse_conn *fc, struct user_namespace *user_ns)
 {
 	memset(fc, 0, sizeof(*fc));
 	spin_lock_init(&fc->lock);
@@ -611,6 +614,7 @@ void fuse_conn_init(struct fuse_conn *fc)
 	fc->attr_version = 1;
 	get_random_bytes(&fc->scramble_key, sizeof(fc->scramble_key));
 	fc->pid_ns = get_pid_ns(task_active_pid_ns(current));
+	fc->user_ns = get_user_ns(user_ns);
 }
 EXPORT_SYMBOL_GPL(fuse_conn_init);
 
@@ -620,6 +624,7 @@ void fuse_conn_put(struct fuse_conn *fc)
 		if (fc->destroy_req)
 			fuse_request_free(fc->destroy_req);
 		put_pid_ns(fc->pid_ns);
+		put_user_ns(fc->user_ns);
 		fc->release(fc);
 	}
 }
@@ -1046,7 +1051,7 @@ static int fuse_fill_super(struct super_block *sb, void *data, int silent)
 
 	sb->s_flags &= ~(MS_NOSEC | MS_I_VERSION);
 
-	if (!parse_fuse_opt(data, &d, is_bdev))
+	if (!parse_fuse_opt(data, &d, is_bdev, sb->s_user_ns))
 		goto err;
 
 	if (is_bdev) {
@@ -1070,8 +1075,12 @@ static int fuse_fill_super(struct super_block *sb, void *data, int silent)
 	if (!file)
 		goto err;
 
-	if ((file->f_op != &fuse_dev_operations) ||
-	    (file->f_cred->user_ns != &init_user_ns))
+	/*
+	 * Require mount to happen from the same user namespace which
+	 * opened /dev/fuse to prevent potential attacks.
+	 */
+	if (file->f_op != &fuse_dev_operations ||
+	    file->f_cred->user_ns != sb->s_user_ns)
 		goto err_fput;
 
 	fc = kmalloc(sizeof(*fc), GFP_KERNEL);
@@ -1079,7 +1088,7 @@ static int fuse_fill_super(struct super_block *sb, void *data, int silent)
 	if (!fc)
 		goto err_fput;
 
-	fuse_conn_init(fc);
+	fuse_conn_init(fc, sb->s_user_ns);
 	fc->release = fuse_free_conn;
 
 	fud = fuse_dev_alloc(fc);
-- 
2.7.4


------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
-- 
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel

^ permalink raw reply related

* [PATCH v4 18/21] fuse: Add support for pid namespaces
From: Seth Forshee @ 2016-04-26 19:36 UTC (permalink / raw)
  To: Eric W. Biederman, Miklos Szeredi
  Cc: linux-bcache-u79uwXL29TY76Z2rM5mHXA, Serge Hallyn, Seth Forshee,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA, Miklos Szeredi,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Austin S Hemmelgarn,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Alexander Viro,
	selinux-+05T5uksL2qpZYMLLGbcSA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	cgroups-u79uwXL29TY76Z2rM5mHXA, Pavel Tikhomirov
In-Reply-To: <1461699396-33000-1-git-send-email-seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

When the userspace process servicing fuse requests is running in
a pid namespace then pids passed via the fuse fd are not being
translated into that process' namespace. Translation is necessary
for the pid to be useful to that process.

Since no use case currently exists for changing namespaces all
translations can be done relative to the pid namespace in use
when fuse_conn_init() is called. For fuse this translates to
mount time, and for cuse this is when /dev/cuse is opened. IO for
this connection from another namespace will return errors.

Requests from processes whose pid cannot be translated into the
target namespace are not permitted, except for requests
allocated via fuse_get_req_nofail_nopages. For no-fail requests
in.h.pid will be 0 if the pid translation fails.

File locking changes based on previous work done by Eric
Biederman.

Signed-off-by: Seth Forshee <seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Acked-by: Miklos Szeredi <mszeredi-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 fs/fuse/dev.c    | 19 +++++++++++++++----
 fs/fuse/file.c   | 22 +++++++++++++++++-----
 fs/fuse/fuse_i.h |  4 ++++
 fs/fuse/inode.c  |  3 +++
 4 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index cbece1221417..4e91b2ac25a7 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -19,6 +19,7 @@
 #include <linux/pipe_fs_i.h>
 #include <linux/swap.h>
 #include <linux/splice.h>
+#include <linux/sched.h>
 
 MODULE_ALIAS_MISCDEV(FUSE_MINOR);
 MODULE_ALIAS("devname:fuse");
@@ -124,11 +125,11 @@ static void __fuse_put_request(struct fuse_req *req)
 	atomic_dec(&req->count);
 }
 
-static void fuse_req_init_context(struct fuse_req *req)
+static void fuse_req_init_context(struct fuse_conn *fc, struct fuse_req *req)
 {
 	req->in.h.uid = from_kuid_munged(&init_user_ns, current_fsuid());
 	req->in.h.gid = from_kgid_munged(&init_user_ns, current_fsgid());
-	req->in.h.pid = current->pid;
+	req->in.h.pid = pid_nr_ns(task_pid(current), fc->pid_ns);
 }
 
 void fuse_set_initialized(struct fuse_conn *fc)
@@ -181,10 +182,14 @@ static struct fuse_req *__fuse_get_req(struct fuse_conn *fc, unsigned npages,
 		goto out;
 	}
 
-	fuse_req_init_context(req);
+	fuse_req_init_context(fc, req);
 	__set_bit(FR_WAITING, &req->flags);
 	if (for_background)
 		__set_bit(FR_BACKGROUND, &req->flags);
+	if (req->in.h.pid == 0) {
+		fuse_put_request(fc, req);
+		return ERR_PTR(-EOVERFLOW);
+	}
 
 	return req;
 
@@ -274,7 +279,7 @@ struct fuse_req *fuse_get_req_nofail_nopages(struct fuse_conn *fc,
 	if (!req)
 		req = get_reserved_req(fc, file);
 
-	fuse_req_init_context(req);
+	fuse_req_init_context(fc, req);
 	__set_bit(FR_WAITING, &req->flags);
 	__clear_bit(FR_BACKGROUND, &req->flags);
 	return req;
@@ -1243,6 +1248,9 @@ static ssize_t fuse_dev_do_read(struct fuse_dev *fud, struct file *file,
 	struct fuse_in *in;
 	unsigned reqsize;
 
+	if (task_active_pid_ns(current) != fc->pid_ns)
+		return -EIO;
+
  restart:
 	spin_lock(&fiq->waitq.lock);
 	err = -EAGAIN;
@@ -1872,6 +1880,9 @@ static ssize_t fuse_dev_do_write(struct fuse_dev *fud,
 	struct fuse_req *req;
 	struct fuse_out_header oh;
 
+	if (task_active_pid_ns(current) != fc->pid_ns)
+		return -EIO;
+
 	if (nbytes < sizeof(struct fuse_out_header))
 		return -EINVAL;
 
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 719924d6c706..b5c616c5ec98 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -2067,7 +2067,8 @@ static int fuse_direct_mmap(struct file *file, struct vm_area_struct *vma)
 	return generic_file_mmap(file, vma);
 }
 
-static int convert_fuse_file_lock(const struct fuse_file_lock *ffl,
+static int convert_fuse_file_lock(struct fuse_conn *fc,
+				  const struct fuse_file_lock *ffl,
 				  struct file_lock *fl)
 {
 	switch (ffl->type) {
@@ -2082,7 +2083,14 @@ static int convert_fuse_file_lock(const struct fuse_file_lock *ffl,
 
 		fl->fl_start = ffl->start;
 		fl->fl_end = ffl->end;
-		fl->fl_pid = ffl->pid;
+
+		/*
+		 * Convert pid into the caller's pid namespace. If the pid
+		 * does not map into the namespace fl_pid will get set to 0.
+		 */
+		rcu_read_lock();
+		fl->fl_pid = pid_vnr(find_pid_ns(ffl->pid, fc->pid_ns));
+		rcu_read_unlock();
 		break;
 
 	default:
@@ -2131,7 +2139,7 @@ static int fuse_getlk(struct file *file, struct file_lock *fl)
 	args.out.args[0].value = &outarg;
 	err = fuse_simple_request(fc, &args);
 	if (!err)
-		err = convert_fuse_file_lock(&outarg.lk, fl);
+		err = convert_fuse_file_lock(fc, &outarg.lk, fl);
 
 	return err;
 }
@@ -2143,7 +2151,8 @@ static int fuse_setlk(struct file *file, struct file_lock *fl, int flock)
 	FUSE_ARGS(args);
 	struct fuse_lk_in inarg;
 	int opcode = (fl->fl_flags & FL_SLEEP) ? FUSE_SETLKW : FUSE_SETLK;
-	pid_t pid = fl->fl_type != F_UNLCK ? current->tgid : 0;
+	struct pid *pid = fl->fl_type != F_UNLCK ? task_tgid(current) : NULL;
+	pid_t pid_nr = pid_nr_ns(pid, fc->pid_ns);
 	int err;
 
 	if (fl->fl_lmops && fl->fl_lmops->lm_grant) {
@@ -2155,7 +2164,10 @@ static int fuse_setlk(struct file *file, struct file_lock *fl, int flock)
 	if (fl->fl_flags & FL_CLOSE)
 		return 0;
 
-	fuse_lk_fill(&args, file, fl, opcode, pid, flock, &inarg);
+	if (pid && pid_nr == 0)
+		return -EOVERFLOW;
+
+	fuse_lk_fill(&args, file, fl, opcode, pid_nr, flock, &inarg);
 	err = fuse_simple_request(fc, &args);
 
 	/* locking is restartable */
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index eddbe02c4028..9145445a759a 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -23,6 +23,7 @@
 #include <linux/poll.h>
 #include <linux/workqueue.h>
 #include <linux/kref.h>
+#include <linux/pid_namespace.h>
 
 /** Max number of pages that can be used in a single read request */
 #define FUSE_MAX_PAGES_PER_REQ 32
@@ -465,6 +466,9 @@ struct fuse_conn {
 	/** The group id for this mount */
 	kgid_t group_id;
 
+	/** The pid namespace for this mount */
+	struct pid_namespace *pid_ns;
+
 	/** The fuse mount flags for this mount */
 	unsigned flags;
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 1ce67668a8e1..eade0bfa4488 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -20,6 +20,7 @@
 #include <linux/random.h>
 #include <linux/sched.h>
 #include <linux/exportfs.h>
+#include <linux/pid_namespace.h>
 
 MODULE_AUTHOR("Miklos Szeredi <miklos-sUDqSbJrdHQHWmgEVkV9KA@public.gmane.org>");
 MODULE_DESCRIPTION("Filesystem in Userspace");
@@ -609,6 +610,7 @@ void fuse_conn_init(struct fuse_conn *fc)
 	fc->connected = 1;
 	fc->attr_version = 1;
 	get_random_bytes(&fc->scramble_key, sizeof(fc->scramble_key));
+	fc->pid_ns = get_pid_ns(task_active_pid_ns(current));
 }
 EXPORT_SYMBOL_GPL(fuse_conn_init);
 
@@ -617,6 +619,7 @@ void fuse_conn_put(struct fuse_conn *fc)
 	if (atomic_dec_and_test(&fc->count)) {
 		if (fc->destroy_req)
 			fuse_request_free(fc->destroy_req);
+		put_pid_ns(fc->pid_ns);
 		fc->release(fc);
 	}
 }
-- 
2.7.4


------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
-- 
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel

^ permalink raw reply related

* [PATCH v4 17/21] capabilities: Allow privileged user in s_user_ns to set security.* xattrs
From: Seth Forshee @ 2016-04-26 19:36 UTC (permalink / raw)
  To: Eric W. Biederman, Serge Hallyn, James Morris, Serge E. Hallyn
  Cc: linux-bcache-u79uwXL29TY76Z2rM5mHXA, Miklos Szeredi, Seth Forshee,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Austin S Hemmelgarn,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Alexander Viro,
	selinux-+05T5uksL2qpZYMLLGbcSA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	cgroups-u79uwXL29TY76Z2rM5mHXA, Pavel Tikhomirov
In-Reply-To: <1461699396-33000-1-git-send-email-seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

A privileged user in s_user_ns will generally have the ability to
manipulate the backing store and insert security.* xattrs into
the filesystem directly. Therefore the kernel must be prepared to
handle these xattrs from unprivileged mounts, and it makes little
sense for commoncap to prevent writing these xattrs to the
filesystem. The capability and LSM code have already been updated
to appropriately handle xattrs from unprivileged mounts, so it
is safe to loosen this restriction on setting xattrs.

The exception to this logic is that writing xattrs to a mounted
filesystem may also cause the LSM inode_post_setxattr or
inode_setsecurity callbacks to be invoked. SELinux will deny the
xattr update by virtue of applying mountpoint labeling to
unprivileged userns mounts, and Smack will deny the writes for
any user without global CAP_MAC_ADMIN, so loosening the
capability check in commoncap is safe in this respect as well.

Signed-off-by: Seth Forshee <seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Acked-by: Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
 security/commoncap.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/security/commoncap.c b/security/commoncap.c
index e657227d221e..12477afaa8ed 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -664,15 +664,17 @@ int cap_bprm_secureexec(struct linux_binprm *bprm)
 int cap_inode_setxattr(struct dentry *dentry, const char *name,
 		       const void *value, size_t size, int flags)
 {
+	struct user_namespace *user_ns = dentry->d_sb->s_user_ns;
+
 	if (!strcmp(name, XATTR_NAME_CAPS)) {
-		if (!capable(CAP_SETFCAP))
+		if (!ns_capable(user_ns, CAP_SETFCAP))
 			return -EPERM;
 		return 0;
 	}
 
 	if (!strncmp(name, XATTR_SECURITY_PREFIX,
 		     sizeof(XATTR_SECURITY_PREFIX) - 1) &&
-	    !capable(CAP_SYS_ADMIN))
+	    !ns_capable(user_ns, CAP_SYS_ADMIN))
 		return -EPERM;
 	return 0;
 }
@@ -690,15 +692,17 @@ int cap_inode_setxattr(struct dentry *dentry, const char *name,
  */
 int cap_inode_removexattr(struct dentry *dentry, const char *name)
 {
+	struct user_namespace *user_ns = dentry->d_sb->s_user_ns;
+
 	if (!strcmp(name, XATTR_NAME_CAPS)) {
-		if (!capable(CAP_SETFCAP))
+		if (!ns_capable(user_ns, CAP_SETFCAP))
 			return -EPERM;
 		return 0;
 	}
 
 	if (!strncmp(name, XATTR_SECURITY_PREFIX,
 		     sizeof(XATTR_SECURITY_PREFIX) - 1) &&
-	    !capable(CAP_SYS_ADMIN))
+	    !ns_capable(user_ns, CAP_SYS_ADMIN))
 		return -EPERM;
 	return 0;
 }
-- 
2.7.4


------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
-- 
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel

^ permalink raw reply related

* [PATCH v4 16/21] fs: Allow superblock owner to access do_remount_sb()
From: Seth Forshee @ 2016-04-26 19:36 UTC (permalink / raw)
  To: Eric W. Biederman, Alexander Viro
  Cc: linux-bcache-u79uwXL29TY76Z2rM5mHXA, Serge Hallyn, Seth Forshee,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA, Miklos Szeredi,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Austin S Hemmelgarn,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	selinux-+05T5uksL2qpZYMLLGbcSA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	cgroups-u79uwXL29TY76Z2rM5mHXA, Pavel Tikhomirov
In-Reply-To: <1461699396-33000-1-git-send-email-seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

Superblock level remounts are currently restricted to global
CAP_SYS_ADMIN, as is the path for changing the root mount to
read only on umount. Loosen both of these permission checks to
also allow CAP_SYS_ADMIN in any namespace which is privileged
towards the userns which originally mounted the filesystem.

Signed-off-by: Seth Forshee <seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Acked-by: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
Acked-by: Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
 fs/namespace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index 0ad8e4a4f50b..575e3f8b34fd 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1510,7 +1510,7 @@ static int do_umount(struct mount *mnt, int flags)
 		 * Special case for "unmounting" root ...
 		 * we just try to remount it readonly.
 		 */
-		if (!capable(CAP_SYS_ADMIN))
+		if (!ns_capable(sb->s_user_ns, CAP_SYS_ADMIN))
 			return -EPERM;
 		down_write(&sb->s_umount);
 		if (!(sb->s_flags & MS_RDONLY))
@@ -2207,7 +2207,7 @@ static int do_remount(struct path *path, int flags, int mnt_flags,
 	down_write(&sb->s_umount);
 	if (flags & MS_BIND)
 		err = change_mount_flags(path->mnt, flags);
-	else if (!capable(CAP_SYS_ADMIN))
+	else if (!ns_capable(sb->s_user_ns, CAP_SYS_ADMIN))
 		err = -EPERM;
 	else
 		err = do_remount_sb(sb, flags, data, 0);
-- 
2.7.4


------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
-- 
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel

^ permalink raw reply related

* [PATCH v4 15/21] fs: Don't remove suid for CAP_FSETID in s_user_ns
From: Seth Forshee @ 2016-04-26 19:36 UTC (permalink / raw)
  To: Eric W. Biederman, Alexander Viro
  Cc: linux-bcache, Serge Hallyn, Seth Forshee, dm-devel,
	Miklos Szeredi, Richard Weinberger, linux-security-module,
	linux-kernel, linux-raid, fuse-devel, Austin S Hemmelgarn,
	linux-mtd, selinux, linux-fsdevel, cgroups, Pavel Tikhomirov
In-Reply-To: <1461699396-33000-1-git-send-email-seth.forshee@canonical.com>

Expand the check in should_remove_suid() to keep privileges for
CAP_FSETID in s_user_ns rather than init_user_ns.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
---
 fs/inode.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/inode.c b/fs/inode.c
index 69b8b526c194..cd52170f9117 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1690,7 +1690,8 @@ int should_remove_suid(struct dentry *dentry)
 	if (unlikely((mode & S_ISGID) && (mode & S_IXGRP)))
 		kill |= ATTR_KILL_SGID;
 
-	if (unlikely(kill && !capable(CAP_FSETID) && S_ISREG(mode)))
+	if (unlikely(kill && !ns_capable(dentry->d_sb->s_user_ns, CAP_FSETID) &&
+		     S_ISREG(mode)))
 		return kill;
 
 	return 0;
-- 
2.7.4


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

^ permalink raw reply related

* [PATCH v4 14/21] fs: Allow superblock owner to change ownership of inodes with unmappable ids
From: Seth Forshee @ 2016-04-26 19:36 UTC (permalink / raw)
  To: Eric W. Biederman, Alexander Viro, Greg Kroah-Hartman
  Cc: linux-bcache-u79uwXL29TY76Z2rM5mHXA, Serge Hallyn, Seth Forshee,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA, Miklos Szeredi,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Austin S Hemmelgarn,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	selinux-+05T5uksL2qpZYMLLGbcSA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	cgroups-u79uwXL29TY76Z2rM5mHXA, Pavel Tikhomirov
In-Reply-To: <1461699396-33000-1-git-send-email-seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

In a userns mount some on-disk inodes may have ids which do not
map into s_user_ns, in which case the in-kernel inodes are owned
by invalid users. The superblock owner should be able to change
attributes of these inodes but cannot. However it is unsafe to
grant the superblock owner privileged access to all inodes in the
superblock since proc, sysfs, etc. use DAC to protect files which
may not belong to s_user_ns. The problem is restricted to only
inodes where the owner or group is an invalid user.

We can work around this by allowing users with CAP_CHOWN in
s_user_ns to change an invalid owner or group id, so long as the
other id is either invalid or mappable in s_user_ns. After
changing ownership the user will be privileged towards the inode
and thus able to change other attributes.

As an precaution, checks for invalid ids are added to the proc
and kernfs setattr interfaces. These filesystems are not expected
to have inodes with invalid ids, but if it does happen any
setattr operations will return -EPERM.

Signed-off-by: Seth Forshee <seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Acked-by: Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
 fs/attr.c             | 62 ++++++++++++++++++++++++++++++++++++++++++++-------
 fs/kernfs/inode.c     |  2 ++
 fs/proc/base.c        |  2 ++
 fs/proc/generic.c     |  3 +++
 fs/proc/proc_sysctl.c |  2 ++
 5 files changed, 63 insertions(+), 8 deletions(-)

diff --git a/fs/attr.c b/fs/attr.c
index 3cfaaac4a18e..06bb3f401559 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -16,6 +16,58 @@
 #include <linux/evm.h>
 #include <linux/ima.h>
 
+static bool chown_ok(const struct inode *inode, kuid_t uid)
+{
+	struct user_namespace *user_ns;
+
+	if (uid_eq(current_fsuid(), inode->i_uid) && uid_eq(uid, inode->i_uid))
+		return true;
+	if (capable_wrt_inode_uidgid(inode, CAP_CHOWN))
+		return true;
+
+	/*
+	 * Inode uids/gids are of type kuid_t/kgid_t. As such, they can be
+	 * a) INVALID_UID/INVALID_GID, b) valid and mappable into
+	 * i_sb->s_user_ns, or c) valid but not mappable into
+	 * i_sb->s_user_ns.
+	 *
+	 * For filesystems on user-supplied media ids will either be (a) or
+	 * (b), so we permit CAP_CHOWN in s_user_ns to change INVALID_UID if
+	 * the gid meets these conditions (and vice versa for INVALID_GID).
+	 *
+	 * For psuedo-filesystems like proc or sysfs ids will be either (b)
+	 * or (c), so these conditions do not permit namespace-root to chown
+	 * in those filesystems.
+	 */
+	user_ns = inode->i_sb->s_user_ns;
+	if (!uid_valid(inode->i_uid) &&
+	    (!gid_valid(inode->i_gid) || kgid_has_mapping(user_ns, inode->i_gid)) &&
+	    ns_capable(user_ns, CAP_CHOWN))
+		return true;
+
+	return false;
+}
+
+static bool chgrp_ok(const struct inode *inode, kgid_t gid)
+{
+	struct user_namespace *user_ns;
+
+	if (uid_eq(current_fsuid(), inode->i_uid) &&
+	    (in_group_p(gid) || gid_eq(gid, inode->i_gid)))
+		return true;
+	if (capable_wrt_inode_uidgid(inode, CAP_CHOWN))
+		return true;
+
+	/* Logic here is the same as in chown_ok(); see comment there. */
+	user_ns = inode->i_sb->s_user_ns;
+	if (!gid_valid(inode->i_gid) &&
+	    (!uid_valid(inode->i_uid) || kuid_has_mapping(user_ns, inode->i_uid)) &&
+	    ns_capable(user_ns, CAP_CHOWN))
+		return true;
+
+	return false;
+}
+
 /**
  * inode_change_ok - check if attribute changes to an inode are allowed
  * @inode:	inode to check
@@ -58,17 +110,11 @@ int inode_change_ok(const struct inode *inode, struct iattr *attr)
 		return 0;
 
 	/* Make sure a caller can chown. */
-	if ((ia_valid & ATTR_UID) &&
-	    (!uid_eq(current_fsuid(), inode->i_uid) ||
-	     !uid_eq(attr->ia_uid, inode->i_uid)) &&
-	    !capable_wrt_inode_uidgid(inode, CAP_CHOWN))
+	if ((ia_valid & ATTR_UID) && !chown_ok(inode, attr->ia_uid))
 		return -EPERM;
 
 	/* Make sure caller can chgrp. */
-	if ((ia_valid & ATTR_GID) &&
-	    (!uid_eq(current_fsuid(), inode->i_uid) ||
-	    (!in_group_p(attr->ia_gid) && !gid_eq(attr->ia_gid, inode->i_gid))) &&
-	    !capable_wrt_inode_uidgid(inode, CAP_CHOWN))
+	if ((ia_valid & ATTR_GID) && !chgrp_ok(inode, attr->ia_gid))
 		return -EPERM;
 
 	/* Make sure a caller can chmod. */
diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
index 16405ae88d2d..2e97a337ee5f 100644
--- a/fs/kernfs/inode.c
+++ b/fs/kernfs/inode.c
@@ -117,6 +117,8 @@ int kernfs_iop_setattr(struct dentry *dentry, struct iattr *iattr)
 
 	if (!kn)
 		return -EINVAL;
+	if (!uid_valid(inode->i_uid) || !gid_valid(inode->i_gid))
+		return -EPERM;
 
 	mutex_lock(&kernfs_mutex);
 	error = inode_change_ok(inode, iattr);
diff --git a/fs/proc/base.c b/fs/proc/base.c
index b1755b23893e..648d623e2158 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -711,6 +711,8 @@ int proc_setattr(struct dentry *dentry, struct iattr *attr)
 
 	if (attr->ia_valid & ATTR_MODE)
 		return -EPERM;
+	if (!uid_valid(inode->i_uid) || !gid_valid(inode->i_gid))
+		return -EPERM;
 
 	error = inode_change_ok(inode, attr);
 	if (error)
diff --git a/fs/proc/generic.c b/fs/proc/generic.c
index ff3ffc76a937..1461570c552c 100644
--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -105,6 +105,9 @@ static int proc_notify_change(struct dentry *dentry, struct iattr *iattr)
 	struct proc_dir_entry *de = PDE(inode);
 	int error;
 
+	if (!uid_valid(inode->i_uid) || !gid_valid(inode->i_gid))
+		return -EPERM;
+
 	error = inode_change_ok(inode, iattr);
 	if (error)
 		return error;
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index fe5b6e6c4671..f5d575157194 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -752,6 +752,8 @@ static int proc_sys_setattr(struct dentry *dentry, struct iattr *attr)
 
 	if (attr->ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID))
 		return -EPERM;
+	if (!uid_valid(inode->i_uid) || !gid_valid(inode->i_gid))
+		return -EPERM;
 
 	error = inode_change_ok(inode, attr);
 	if (error)
-- 
2.7.4


------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
-- 
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel

^ permalink raw reply related

* [PATCH v4 13/21] fs: Update posix_acl support to handle user namespace mounts
From: Seth Forshee @ 2016-04-26 19:36 UTC (permalink / raw)
  To: Eric W. Biederman, Alexander Viro
  Cc: linux-bcache-u79uwXL29TY76Z2rM5mHXA, Serge Hallyn, Seth Forshee,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA, Miklos Szeredi,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Austin S Hemmelgarn,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	selinux-+05T5uksL2qpZYMLLGbcSA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	cgroups-u79uwXL29TY76Z2rM5mHXA, Pavel Tikhomirov
In-Reply-To: <1461699396-33000-1-git-send-email-seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

ids in on-disk ACLs should be converted to s_user_ns instead of
init_user_ns as is done now. This introduces the possibility for
id mappings to fail, and when this happens syscalls will return
EOVERFLOW.

Signed-off-by: Seth Forshee <seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Acked-by: Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
 fs/posix_acl.c                  | 67 ++++++++++++++++++++++++++---------------
 fs/xattr.c                      | 19 +++++++++---
 include/linux/posix_acl_xattr.h | 17 ++++++++---
 3 files changed, 70 insertions(+), 33 deletions(-)

diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index 711dd5170376..dac2842dd4cb 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -595,59 +595,77 @@ EXPORT_SYMBOL_GPL(posix_acl_create);
 /*
  * Fix up the uids and gids in posix acl extended attributes in place.
  */
-static void posix_acl_fix_xattr_userns(
+static int posix_acl_fix_xattr_userns(
 	struct user_namespace *to, struct user_namespace *from,
 	void *value, size_t size)
 {
 	posix_acl_xattr_header *header = (posix_acl_xattr_header *)value;
 	posix_acl_xattr_entry *entry = (posix_acl_xattr_entry *)(header+1), *end;
 	int count;
-	kuid_t uid;
-	kgid_t gid;
+	kuid_t kuid;
+	uid_t uid;
+	kgid_t kgid;
+	gid_t gid;
 
 	if (!value)
-		return;
+		return 0;
 	if (size < sizeof(posix_acl_xattr_header))
-		return;
+		return 0;
 	if (header->a_version != cpu_to_le32(POSIX_ACL_XATTR_VERSION))
-		return;
+		return 0;
 
 	count = posix_acl_xattr_count(size);
 	if (count < 0)
-		return;
+		return 0;
 	if (count == 0)
-		return;
+		return 0;
 
 	for (end = entry + count; entry != end; entry++) {
 		switch(le16_to_cpu(entry->e_tag)) {
 		case ACL_USER:
-			uid = make_kuid(from, le32_to_cpu(entry->e_id));
-			entry->e_id = cpu_to_le32(from_kuid(to, uid));
+			kuid = make_kuid(from, le32_to_cpu(entry->e_id));
+			if (!uid_valid(kuid))
+				return -EOVERFLOW;
+			uid = from_kuid(to, kuid);
+			if (uid == (uid_t)-1)
+				return -EOVERFLOW;
+			entry->e_id = cpu_to_le32(uid);
 			break;
 		case ACL_GROUP:
-			gid = make_kgid(from, le32_to_cpu(entry->e_id));
-			entry->e_id = cpu_to_le32(from_kgid(to, gid));
+			kgid = make_kgid(from, le32_to_cpu(entry->e_id));
+			if (!gid_valid(kgid))
+				return -EOVERFLOW;
+			gid = from_kgid(to, kgid);
+			if (gid == (gid_t)-1)
+				return -EOVERFLOW;
+			entry->e_id = cpu_to_le32(gid);
 			break;
 		default:
 			break;
 		}
 	}
+
+	return 0;
 }
 
-void posix_acl_fix_xattr_from_user(void *value, size_t size)
+int
+posix_acl_fix_xattr_from_user(struct user_namespace *target_ns, void *value,
+			      size_t size)
 {
-	struct user_namespace *user_ns = current_user_ns();
-	if (user_ns == &init_user_ns)
-		return;
-	posix_acl_fix_xattr_userns(&init_user_ns, user_ns, value, size);
+	struct user_namespace *source_ns = current_user_ns();
+	if (source_ns == target_ns)
+		return 0;
+	return posix_acl_fix_xattr_userns(target_ns, source_ns, value, size);
 }
 
-void posix_acl_fix_xattr_to_user(void *value, size_t size)
+int
+posix_acl_fix_xattr_to_user(struct user_namespace *source_ns, void *value,
+			    size_t size)
 {
-	struct user_namespace *user_ns = current_user_ns();
-	if (user_ns == &init_user_ns)
-		return;
-	posix_acl_fix_xattr_userns(user_ns, &init_user_ns, value, size);
+	struct user_namespace *target_ns = current_user_ns();
+	if (target_ns == source_ns)
+		return 0;
+	return posix_acl_fix_xattr_userns(target_ns, source_ns, value, size);
 }
 
 /*
@@ -780,7 +798,7 @@ posix_acl_xattr_get(const struct xattr_handler *handler,
 	if (acl == NULL)
 		return -ENODATA;
 
-	error = posix_acl_to_xattr(&init_user_ns, acl, value, size);
+	error = posix_acl_to_xattr(dentry->d_sb->s_user_ns, acl, value, size);
 	posix_acl_release(acl);
 
 	return error;
@@ -806,7 +824,8 @@ posix_acl_xattr_set(const struct xattr_handler *handler,
 		return -EPERM;
 
 	if (value) {
-		acl = posix_acl_from_xattr(&init_user_ns, value, size);
+		acl = posix_acl_from_xattr(dentry->d_sb->s_user_ns, value,
+					   size);
 		if (IS_ERR(acl))
 			return PTR_ERR(acl);
 
diff --git a/fs/xattr.c b/fs/xattr.c
index 4861322e28e8..c541121945cd 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -330,8 +330,12 @@ setxattr(struct dentry *d, const char __user *name, const void __user *value,
 			goto out;
 		}
 		if ((strcmp(kname, XATTR_NAME_POSIX_ACL_ACCESS) == 0) ||
-		    (strcmp(kname, XATTR_NAME_POSIX_ACL_DEFAULT) == 0))
-			posix_acl_fix_xattr_from_user(kvalue, size);
+		    (strcmp(kname, XATTR_NAME_POSIX_ACL_DEFAULT) == 0)) {
+			error = posix_acl_fix_xattr_from_user(d->d_sb->s_user_ns,
+							      kvalue, size);
+			if (error)
+				goto out;
+		}
 	}
 
 	error = vfs_setxattr(d, kname, kvalue, size, flags);
@@ -427,9 +431,14 @@ getxattr(struct dentry *d, const char __user *name, void __user *value,
 	error = vfs_getxattr(d, kname, kvalue, size);
 	if (error > 0) {
 		if ((strcmp(kname, XATTR_NAME_POSIX_ACL_ACCESS) == 0) ||
-		    (strcmp(kname, XATTR_NAME_POSIX_ACL_DEFAULT) == 0))
-			posix_acl_fix_xattr_to_user(kvalue, size);
-		if (size && copy_to_user(value, kvalue, error))
+		    (strcmp(kname, XATTR_NAME_POSIX_ACL_DEFAULT) == 0)) {
+			int ret;
+			ret = posix_acl_fix_xattr_to_user(d->d_sb->s_user_ns,
+							  kvalue, size);
+			if (ret)
+				error = ret;
+		}
+		if (error > 0 && size && copy_to_user(value, kvalue, error))
 			error = -EFAULT;
 	} else if (error == -ERANGE && size >= XATTR_SIZE_MAX) {
 		/* The file system tried to returned a value bigger
diff --git a/include/linux/posix_acl_xattr.h b/include/linux/posix_acl_xattr.h
index e5e8ec40278d..5dec6b10951a 100644
--- a/include/linux/posix_acl_xattr.h
+++ b/include/linux/posix_acl_xattr.h
@@ -49,14 +49,23 @@ posix_acl_xattr_count(size_t size)
 }
 
 #ifdef CONFIG_FS_POSIX_ACL
-void posix_acl_fix_xattr_from_user(void *value, size_t size);
-void posix_acl_fix_xattr_to_user(void *value, size_t size);
+int posix_acl_fix_xattr_from_user(struct user_namespace *target_ns,
+				  void *value, size_t size);
+int posix_acl_fix_xattr_to_user(struct user_namespace *source_ns, void *value,
+				size_t size);
 #else
-static inline void posix_acl_fix_xattr_from_user(void *value, size_t size)
+static inline int
+posix_acl_fix_xattr_from_user(struct user_namespace *target_ns, void *value,
+			      size_t size)
 {
+	return 0;
 }
-static inline void posix_acl_fix_xattr_to_user(void *value, size_t size)
+
+static inline int
+posix_acl_fix_xattr_to_user(struct user_namespace *source_ns, void *value,
+			    size_t size)
 {
+	return 0;
 }
 #endif
 
-- 
2.7.4


------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
-- 
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel

^ permalink raw reply related

* [PATCH v4 12/21] fs: Refuse uid/gid changes which don't map into s_user_ns
From: Seth Forshee @ 2016-04-26 19:36 UTC (permalink / raw)
  To: Eric W. Biederman, Alexander Viro
  Cc: linux-bcache-u79uwXL29TY76Z2rM5mHXA, Serge Hallyn, Seth Forshee,
	dm-devel-H+wXaHxf7aLQT0dZR+AlfA, Miklos Szeredi,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-raid-u79uwXL29TY76Z2rM5mHXA,
	fuse-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Austin S Hemmelgarn,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	selinux-+05T5uksL2qpZYMLLGbcSA,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	cgroups-u79uwXL29TY76Z2rM5mHXA, Pavel Tikhomirov
In-Reply-To: <1461699396-33000-1-git-send-email-seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

Add checks to inode_change_ok to verify that uid and gid changes
will map into the superblock's user namespace. If they do not
fail with -EOVERFLOW. This cannot be overriden with ATTR_FORCE.

Signed-off-by: Seth Forshee <seth.forshee-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
Acked-by: Serge Hallyn <serge.hallyn-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
 fs/attr.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/fs/attr.c b/fs/attr.c
index 25b24d0f6c88..3cfaaac4a18e 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -42,6 +42,17 @@ int inode_change_ok(const struct inode *inode, struct iattr *attr)
 			return error;
 	}
 
+	/*
+	 * Verify that uid/gid changes are valid in the target namespace
+	 * of the superblock. This cannot be overriden using ATTR_FORCE.
+	 */
+	if (ia_valid & ATTR_UID &&
+	    from_kuid(inode->i_sb->s_user_ns, attr->ia_uid) == (uid_t)-1)
+		return -EOVERFLOW;
+	if (ia_valid & ATTR_GID &&
+	    from_kgid(inode->i_sb->s_user_ns, attr->ia_gid) == (gid_t)-1)
+		return -EOVERFLOW;
+
 	/* If force is set do it anyway. */
 	if (ia_valid & ATTR_FORCE)
 		return 0;
-- 
2.7.4


------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
-- 
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel

^ permalink raw reply related

* [PATCH v4 11/21] cred: Reject inodes with invalid ids in set_create_file_as()
From: Seth Forshee @ 2016-04-26 19:36 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: linux-bcache, Serge Hallyn, Seth Forshee, dm-devel,
	Miklos Szeredi, Richard Weinberger, linux-security-module,
	linux-kernel, linux-raid, fuse-devel, Austin S Hemmelgarn,
	linux-mtd, Alexander Viro, selinux, linux-fsdevel, cgroups,
	Pavel Tikhomirov
In-Reply-To: <1461699396-33000-1-git-send-email-seth.forshee@canonical.com>

Using INVALID_[UG]ID for the LSM file creation context doesn't
make sense, so return an error if the inode passed to
set_create_file_as() has an invalid id.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
---
 kernel/cred.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/cred.c b/kernel/cred.c
index 0c0cd8a62285..5f264fb5737d 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -689,6 +689,8 @@ EXPORT_SYMBOL(set_security_override_from_ctx);
  */
 int set_create_files_as(struct cred *new, struct inode *inode)
 {
+	if (!uid_valid(inode->i_uid) || !gid_valid(inode->i_gid))
+		return -EINVAL;
 	new->fsuid = inode->i_uid;
 	new->fsgid = inode->i_gid;
 	return security_kernel_create_files_as(new, inode);
-- 
2.7.4


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

^ permalink raw reply related

* [PATCH v4 10/21] fs: Check for invalid i_uid in may_follow_link()
From: Seth Forshee @ 2016-04-26 19:36 UTC (permalink / raw)
  To: Eric W. Biederman, Alexander Viro
  Cc: linux-bcache, Serge Hallyn, Seth Forshee, dm-devel,
	Miklos Szeredi, Richard Weinberger, linux-security-module,
	linux-kernel, linux-raid, fuse-devel, Austin S Hemmelgarn,
	linux-mtd, selinux, linux-fsdevel, cgroups, Pavel Tikhomirov
In-Reply-To: <1461699396-33000-1-git-send-email-seth.forshee@canonical.com>

Filesystem uids which don't map into a user namespace may result
in inode->i_uid being INVALID_UID. A symlink and its parent
could have different owners in the filesystem can both get
mapped to INVALID_UID, which may result in following a symlink
when this would not have otherwise been permitted when protected
symlinks are enabled.

Add a new helper function, uid_valid_eq(), and use this to
validate that the ids in may_follow_link() are both equal and
valid. Also add an equivalent helper for gids, which is
currently unused.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
---
 fs/namei.c             |  2 +-
 include/linux/uidgid.h | 10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/namei.c b/fs/namei.c
index a29094c6f4a1..6fe8b0d8ca90 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -915,7 +915,7 @@ static inline int may_follow_link(struct nameidata *nd)
 		return 0;
 
 	/* Allowed if parent directory and link owner match. */
-	if (uid_eq(parent->i_uid, inode->i_uid))
+	if (uid_valid_eq(parent->i_uid, inode->i_uid))
 		return 0;
 
 	if (nd->flags & LOOKUP_RCU)
diff --git a/include/linux/uidgid.h b/include/linux/uidgid.h
index 03835522dfcb..e09529fe2668 100644
--- a/include/linux/uidgid.h
+++ b/include/linux/uidgid.h
@@ -117,6 +117,16 @@ static inline bool gid_valid(kgid_t gid)
 	return __kgid_val(gid) != (gid_t) -1;
 }
 
+static inline bool uid_valid_eq(kuid_t left, kuid_t right)
+{
+	return uid_eq(left, right) && uid_valid(left);
+}
+
+static inline bool gid_valid_eq(kgid_t left, kgid_t right)
+{
+	return gid_eq(left, right) && gid_valid(left);
+}
+
 #ifdef CONFIG_USER_NS
 
 extern kuid_t make_kuid(struct user_namespace *from, uid_t uid);
-- 
2.7.4


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

^ permalink raw reply related

* [PATCH v4 09/21] Smack: Handle labels consistently in untrusted mounts
From: Seth Forshee @ 2016-04-26 19:36 UTC (permalink / raw)
  To: Eric W. Biederman, Casey Schaufler
  Cc: Alexander Viro, Serge Hallyn, Richard Weinberger,
	Austin S Hemmelgarn, Miklos Szeredi, Pavel Tikhomirov,
	linux-kernel, linux-bcache, dm-devel, linux-raid, linux-mtd,
	linux-fsdevel, fuse-devel, linux-security-module, selinux,
	cgroups, Seth Forshee, James Morris, Serge E. Hallyn
In-Reply-To: <1461699396-33000-1-git-send-email-seth.forshee@canonical.com>

The SMACK64, SMACK64EXEC, and SMACK64MMAP labels are all handled
differently in untrusted mounts. This is confusing and
potentically problematic. Change this to handle them all the same
way that SMACK64 is currently handled; that is, read the label
from disk and check it at use time. For SMACK64 and SMACK64MMAP
access is denied if the label does not match smk_root. To be
consistent with suid, a SMACK64EXEC label which does not match
smk_root will still allow execution of the file but will not run
with the label supplied in the xattr.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
Acked-by: Casey Schaufler <casey@schaufler-ca.com>
---
 security/smack/smack_lsm.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index aa17198cd5f2..ca564590cc1b 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -919,6 +919,7 @@ static int smack_bprm_set_creds(struct linux_binprm *bprm)
 	struct inode *inode = file_inode(bprm->file);
 	struct task_smack *bsp = bprm->cred->security;
 	struct inode_smack *isp;
+	struct superblock_smack *sbsp;
 	int rc;
 
 	if (bprm->cred_prepared)
@@ -928,6 +929,11 @@ static int smack_bprm_set_creds(struct linux_binprm *bprm)
 	if (isp->smk_task == NULL || isp->smk_task == bsp->smk_task)
 		return 0;
 
+	sbsp = inode->i_sb->s_security;
+	if ((sbsp->smk_flags & SMK_SB_UNTRUSTED) &&
+	    isp->smk_task != sbsp->smk_root)
+		return 0;
+
 	if (bprm->unsafe & (LSM_UNSAFE_PTRACE | LSM_UNSAFE_PTRACE_CAP)) {
 		struct task_struct *tracer;
 		rc = 0;
@@ -1725,6 +1731,7 @@ static int smack_mmap_file(struct file *file,
 	struct task_smack *tsp;
 	struct smack_known *okp;
 	struct inode_smack *isp;
+	struct superblock_smack *sbsp;
 	int may;
 	int mmay;
 	int tmay;
@@ -1736,6 +1743,10 @@ static int smack_mmap_file(struct file *file,
 	isp = file_inode(file)->i_security;
 	if (isp->smk_mmap == NULL)
 		return 0;
+	sbsp = file_inode(file)->i_sb->s_security;
+	if (sbsp->smk_flags & SMK_SB_UNTRUSTED &&
+	    isp->smk_mmap != sbsp->smk_root)
+		return -EACCES;
 	mkp = isp->smk_mmap;
 
 	tsp = current_security();
@@ -3546,16 +3557,14 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
 			if (rc >= 0)
 				transflag = SMK_INODE_TRANSMUTE;
 		}
-		if (!(sbsp->smk_flags & SMK_SB_UNTRUSTED)) {
-			/*
-			 * Don't let the exec or mmap label be "*" or "@".
-			 */
-			skp = smk_fetch(XATTR_NAME_SMACKEXEC, inode, dp);
-			if (IS_ERR(skp) || skp == &smack_known_star ||
-			    skp == &smack_known_web)
-				skp = NULL;
-			isp->smk_task = skp;
-		}
+		/*
+		 * Don't let the exec or mmap label be "*" or "@".
+		 */
+		skp = smk_fetch(XATTR_NAME_SMACKEXEC, inode, dp);
+		if (IS_ERR(skp) || skp == &smack_known_star ||
+		    skp == &smack_known_web)
+			skp = NULL;
+		isp->smk_task = skp;
 
 		skp = smk_fetch(XATTR_NAME_SMACKMMAP, inode, dp);
 		if (IS_ERR(skp) || skp == &smack_known_star ||
-- 
2.7.4

^ permalink raw reply related

* [PATCH v4 08/21] userns: Replace in_userns with current_in_userns
From: Seth Forshee @ 2016-04-26 19:36 UTC (permalink / raw)
  To: Eric W. Biederman, Alexander Viro, Serge Hallyn, James Morris,
	Serge E. Hallyn
  Cc: Miklos Szeredi, Seth Forshee, dm-devel, linux-security-module,
	Richard Weinberger, linux-bcache, linux-kernel, linux-raid,
	fuse-devel, Austin S Hemmelgarn, linux-mtd, selinux,
	linux-fsdevel, cgroups, Pavel Tikhomirov
In-Reply-To: <1461699396-33000-1-git-send-email-seth.forshee@canonical.com>

All current callers of in_userns pass current_user_ns as the
first argument. Simplify by replacing in_userns with
current_in_userns which checks whether current_user_ns is in the
namespace supplied as an argument.

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
Acked-by: James Morris <james.l.morris@oracle.com>
Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
---
 fs/namespace.c                 | 2 +-
 include/linux/user_namespace.h | 6 ++----
 kernel/user_namespace.c        | 6 +++---
 security/commoncap.c           | 2 +-
 4 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index 6e9db4c166b4..0ad8e4a4f50b 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -3294,7 +3294,7 @@ bool mnt_may_suid(struct vfsmount *mnt)
 	 * in other namespaces.
 	 */
 	return !(mnt->mnt_flags & MNT_NOSUID) && check_mnt(real_mount(mnt)) &&
-	       in_userns(current_user_ns(), mnt->mnt_sb->s_user_ns);
+	       current_in_userns(mnt->mnt_sb->s_user_ns);
 }
 
 static struct ns_common *mntns_get(struct task_struct *task)
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index a43faa727124..9217169c64cb 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -72,8 +72,7 @@ extern ssize_t proc_projid_map_write(struct file *, const char __user *, size_t,
 extern ssize_t proc_setgroups_write(struct file *, const char __user *, size_t, loff_t *);
 extern int proc_setgroups_show(struct seq_file *m, void *v);
 extern bool userns_may_setgroups(const struct user_namespace *ns);
-extern bool in_userns(const struct user_namespace *ns,
-		      const struct user_namespace *target_ns);
+extern bool current_in_userns(const struct user_namespace *target_ns);
 #else
 
 static inline struct user_namespace *get_user_ns(struct user_namespace *ns)
@@ -103,8 +102,7 @@ static inline bool userns_may_setgroups(const struct user_namespace *ns)
 	return true;
 }
 
-static inline bool in_userns(const struct user_namespace *ns,
-			     const struct user_namespace *target_ns)
+static inline bool current_in_userns(const struct user_namespace *target_ns)
 {
 	return true;
 }
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index dee3be5445da..68f594212759 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -942,10 +942,10 @@ bool userns_may_setgroups(const struct user_namespace *ns)
  * Returns true if @ns is the same namespace as or a descendant of
  * @target_ns.
  */
-bool in_userns(const struct user_namespace *ns,
-	       const struct user_namespace *target_ns)
+bool current_in_userns(const struct user_namespace *target_ns)
 {
-	for (; ns; ns = ns->parent) {
+	struct user_namespace *ns;
+	for (ns = current_user_ns(); ns; ns = ns->parent) {
 		if (ns == target_ns)
 			return true;
 	}
diff --git a/security/commoncap.c b/security/commoncap.c
index a306c5d90709..e657227d221e 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -461,7 +461,7 @@ static int get_file_caps(struct linux_binprm *bprm, bool *effective, bool *has_c
 	 * explicit that capability bits are limited to s_user_ns and its
 	 * descendants.
 	 */
-	if (!in_userns(current_user_ns(), bprm->file->f_path.mnt->mnt_sb->s_user_ns))
+	if (!current_in_userns(bprm->file->f_path.mnt->mnt_sb->s_user_ns))
 		return 0;
 
 	rc = get_vfs_caps_from_disk(bprm->file->f_path.dentry, &vcaps);
-- 
2.7.4


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

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox