linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] make ubifs compatible with IMA and EVM.
@ 2017-04-11  9:50 Oleksij Rempel
  2017-04-11  9:50 ` [PATCH v2 1/3] fs: ubifs: parse iversion mount option Oleksij Rempel
                   ` (2 more replies)
  0 siblings, 3 replies; 27+ messages in thread
From: Oleksij Rempel @ 2017-04-11  9:50 UTC (permalink / raw)
  To: richard, dedekind1, adrian.hunter, linux-mtd, linux-kernel,
	linux-fsdevel
  Cc: Oleksij Rempel

To make ubifs compatible with IMA nad EVM security modules:
- we need to notify them about inode changes over inode->i_version
- provide super block uuid to allow fs choice by uuid.

Oleksij Rempel (1):
  fs: ubifs: update i_version on inode changes

Steffen Trumtrar (2):
  fs: ubifs: parse iversion mount option
  fs: ubifs: set s_uuid in super block

 fs/ubifs/file.c  | 9 +++++++++
 fs/ubifs/super.c | 8 +++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

-- 
2.11.0

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

* [PATCH v2 1/3] fs: ubifs: parse iversion mount option
  2017-04-11  9:50 [PATCH v2 0/3] make ubifs compatible with IMA and EVM Oleksij Rempel
@ 2017-04-11  9:50 ` Oleksij Rempel
  2017-04-11  9:50 ` [PATCH v2 2/3] fs: ubifs: update i_version on inode changes Oleksij Rempel
  2017-04-11  9:50 ` [PATCH v2 3/3] fs: ubifs: set s_uuid in super block Oleksij Rempel
  2 siblings, 0 replies; 27+ messages in thread
From: Oleksij Rempel @ 2017-04-11  9:50 UTC (permalink / raw)
  To: richard, dedekind1, adrian.hunter, linux-mtd, linux-kernel,
	linux-fsdevel
  Cc: Steffen Trumtrar, Oleksij Rempel

From: Steffen Trumtrar <s.trumtrar@pengutronix.de>

this option is needed to make UBIFS work with IMA.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 fs/ubifs/super.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index b73811bd7676..bff1e8d6f7bd 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -931,6 +931,7 @@ enum {
 	Opt_chk_data_crc,
 	Opt_no_chk_data_crc,
 	Opt_override_compr,
+	Opt_i_version,
 	Opt_err,
 };
 
@@ -942,6 +943,7 @@ static const match_table_t tokens = {
 	{Opt_chk_data_crc, "chk_data_crc"},
 	{Opt_no_chk_data_crc, "no_chk_data_crc"},
 	{Opt_override_compr, "compr=%s"},
+	{Opt_i_version, "i_version"},
 	{Opt_err, NULL},
 };
 
@@ -986,6 +988,7 @@ static int ubifs_parse_options(struct ubifs_info *c, char *options,
 		return 0;
 
 	while ((p = strsep(&options, ","))) {
+		struct super_block *sb = c->vfs_sb;
 		int token;
 
 		if (!*p)
@@ -1042,10 +1045,12 @@ static int ubifs_parse_options(struct ubifs_info *c, char *options,
 			c->default_compr = c->mount_opts.compr_type;
 			break;
 		}
+		case Opt_i_version:
+			sb->s_flags |= MS_I_VERSION;
+			break;
 		default:
 		{
 			unsigned long flag;
-			struct super_block *sb = c->vfs_sb;
 
 			flag = parse_standard_option(p);
 			if (!flag) {
-- 
2.11.0

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

* [PATCH v2 2/3] fs: ubifs: update i_version on inode changes
  2017-04-11  9:50 [PATCH v2 0/3] make ubifs compatible with IMA and EVM Oleksij Rempel
  2017-04-11  9:50 ` [PATCH v2 1/3] fs: ubifs: parse iversion mount option Oleksij Rempel
@ 2017-04-11  9:50 ` Oleksij Rempel
  2017-04-11 16:05   ` Christoph Hellwig
  2017-04-11  9:50 ` [PATCH v2 3/3] fs: ubifs: set s_uuid in super block Oleksij Rempel
  2 siblings, 1 reply; 27+ messages in thread
From: Oleksij Rempel @ 2017-04-11  9:50 UTC (permalink / raw)
  To: richard, dedekind1, adrian.hunter, linux-mtd, linux-kernel,
	linux-fsdevel
  Cc: Oleksij Rempel

increment i_version to notify security/IMA about changes
made in inode.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 fs/ubifs/file.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index d9ae86f96df7..29213724259b 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -1104,6 +1104,8 @@ static void do_attr_changes(struct inode *inode, const struct iattr *attr)
 			mode &= ~S_ISGID;
 		inode->i_mode = mode;
 	}
+	if (IS_I_VERSION(inode))
+		inode_inc_iversion(inode);
 }
 
 /**
@@ -1401,6 +1403,9 @@ int ubifs_update_time(struct inode *inode, struct timespec *time,
 	if (!(inode->i_sb->s_flags & MS_LAZYTIME))
 		iflags |= I_DIRTY_SYNC;
 
+	if (IS_I_VERSION(inode))
+		inode_inc_iversion(inode);
+
 	release = ui->dirty;
 	__mark_inode_dirty(inode, iflags);
 	mutex_unlock(&ui->ui_mutex);
@@ -1435,6 +1440,8 @@ static int update_mctime(struct inode *inode)
 
 		mutex_lock(&ui->ui_mutex);
 		inode->i_mtime = inode->i_ctime = ubifs_current_time(inode);
+		if (IS_I_VERSION(inode))
+			inode_inc_iversion(inode);
 		release = ui->dirty;
 		mark_inode_dirty_sync(inode);
 		mutex_unlock(&ui->ui_mutex);
@@ -1580,6 +1587,8 @@ static int ubifs_vm_page_mkwrite(struct vm_fault *vmf)
 
 		mutex_lock(&ui->ui_mutex);
 		inode->i_mtime = inode->i_ctime = ubifs_current_time(inode);
+		if (IS_I_VERSION(inode))
+			inode_inc_iversion(inode);
 		release = ui->dirty;
 		mark_inode_dirty_sync(inode);
 		mutex_unlock(&ui->ui_mutex);
-- 
2.11.0

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

* [PATCH v2 3/3] fs: ubifs: set s_uuid in super block
  2017-04-11  9:50 [PATCH v2 0/3] make ubifs compatible with IMA and EVM Oleksij Rempel
  2017-04-11  9:50 ` [PATCH v2 1/3] fs: ubifs: parse iversion mount option Oleksij Rempel
  2017-04-11  9:50 ` [PATCH v2 2/3] fs: ubifs: update i_version on inode changes Oleksij Rempel
@ 2017-04-11  9:50 ` Oleksij Rempel
  2017-04-11 20:43   ` Richard Weinberger
  2 siblings, 1 reply; 27+ messages in thread
From: Oleksij Rempel @ 2017-04-11  9:50 UTC (permalink / raw)
  To: richard, dedekind1, adrian.hunter, linux-mtd, linux-kernel,
	linux-fsdevel
  Cc: Steffen Trumtrar, Oleksij Rempel

From: Steffen Trumtrar <s.trumtrar@pengutronix.de>

This is need to provide uuid based integrity functionlity for:
imy_policy (fsuuid option) and  evmctl (--uuid option).

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 fs/ubifs/super.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index bff1e8d6f7bd..a584b2f2b11d 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -2077,6 +2077,7 @@ static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
 		err = -ENOMEM;
 		goto out_umount;
 	}
+	memcpy(&sb->s_uuid, &c->uuid, sizeof(c->uuid));
 
 	mutex_unlock(&c->umount_mutex);
 	return 0;
-- 
2.11.0

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

* Re: [PATCH v2 2/3] fs: ubifs: update i_version on inode changes
  2017-04-11  9:50 ` [PATCH v2 2/3] fs: ubifs: update i_version on inode changes Oleksij Rempel
@ 2017-04-11 16:05   ` Christoph Hellwig
  2017-04-11 21:13     ` Richard Weinberger
  0 siblings, 1 reply; 27+ messages in thread
From: Christoph Hellwig @ 2017-04-11 16:05 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: richard, dedekind1, adrian.hunter, linux-mtd, linux-kernel,
	linux-fsdevel

On Tue, Apr 11, 2017 at 11:50:54AM +0200, Oleksij Rempel wrote:
> increment i_version to notify security/IMA about changes
> made in inode.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>

And how is this stored on disk?

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

* Re: [PATCH v2 3/3] fs: ubifs: set s_uuid in super block
  2017-04-11  9:50 ` [PATCH v2 3/3] fs: ubifs: set s_uuid in super block Oleksij Rempel
@ 2017-04-11 20:43   ` Richard Weinberger
  2017-04-12  5:48     ` Christoph Hellwig
  2017-05-02  7:23     ` Artem Bityutskiy
  0 siblings, 2 replies; 27+ messages in thread
From: Richard Weinberger @ 2017-04-11 20:43 UTC (permalink / raw)
  To: Oleksij Rempel, dedekind1, adrian.hunter, linux-mtd,
	linux-kernel, linux-fsdevel
  Cc: Steffen Trumtrar

Oleksij,

Am 11.04.2017 um 11:50 schrieb Oleksij Rempel:
> From: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> 
> This is need to provide uuid based integrity functionlity for:
> imy_policy (fsuuid option) and  evmctl (--uuid option).
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  fs/ubifs/super.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
> index bff1e8d6f7bd..a584b2f2b11d 100644
> --- a/fs/ubifs/super.c
> +++ b/fs/ubifs/super.c
> @@ -2077,6 +2077,7 @@ static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
>  		err = -ENOMEM;
>  		goto out_umount;
>  	}
> +	memcpy(&sb->s_uuid, &c->uuid, sizeof(c->uuid));

Makes sense.

Artem, do you remember why UBIFS didn't set s_uuid in first place?

Thanks,
//richard

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

* Re: [PATCH v2 2/3] fs: ubifs: update i_version on inode changes
  2017-04-11 16:05   ` Christoph Hellwig
@ 2017-04-11 21:13     ` Richard Weinberger
  2017-04-12  6:05       ` Oleksij Rempel
  0 siblings, 1 reply; 27+ messages in thread
From: Richard Weinberger @ 2017-04-11 21:13 UTC (permalink / raw)
  To: Christoph Hellwig, Oleksij Rempel
  Cc: dedekind1, adrian.hunter, linux-mtd, linux-kernel, linux-fsdevel

Am 11.04.2017 um 18:05 schrieb Christoph Hellwig:
> On Tue, Apr 11, 2017 at 11:50:54AM +0200, Oleksij Rempel wrote:
>> increment i_version to notify security/IMA about changes
>> made in inode.
>>
>> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> 
> And how is this stored on disk?
> 

Hehe, I was about to ask the same question. :-)

Thanks,
//richard

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

* Re: [PATCH v2 3/3] fs: ubifs: set s_uuid in super block
  2017-04-11 20:43   ` Richard Weinberger
@ 2017-04-12  5:48     ` Christoph Hellwig
  2017-04-12  7:15       ` Oleksij Rempel
  2017-05-02  7:23     ` Artem Bityutskiy
  1 sibling, 1 reply; 27+ messages in thread
From: Christoph Hellwig @ 2017-04-12  5:48 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Oleksij Rempel, dedekind1, adrian.hunter, linux-mtd,
	linux-kernel, linux-fsdevel, Steffen Trumtrar

On Tue, Apr 11, 2017 at 10:43:26PM +0200, Richard Weinberger wrote:
> Artem, do you remember why UBIFS didn't set s_uuid in first place?

It's an extremely odd field - only a hand full of file systems set it
(e.g. XFS doesn't, although according to Mimi IMA supports XFS), and
it's never even used outside of the IMA/EVM code.

We really need a feature flag that this field is valid that IMA can
check before adding more support for it.

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

* Re: [PATCH v2 2/3] fs: ubifs: update i_version on inode changes
  2017-04-11 21:13     ` Richard Weinberger
@ 2017-04-12  6:05       ` Oleksij Rempel
  2017-04-12  6:08         ` Christoph Hellwig
  0 siblings, 1 reply; 27+ messages in thread
From: Oleksij Rempel @ 2017-04-12  6:05 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Christoph Hellwig, Oleksij Rempel, dedekind1, adrian.hunter,
	linux-mtd, linux-kernel, linux-fsdevel, kernel

On Tue, Apr 11, 2017 at 11:13:24PM +0200, Richard Weinberger wrote:
> Am 11.04.2017 um 18:05 schrieb Christoph Hellwig:
> > On Tue, Apr 11, 2017 at 11:50:54AM +0200, Oleksij Rempel wrote:
> >> increment i_version to notify security/IMA about changes
> >> made in inode.
> >>
> >> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > 
> > And how is this stored on disk?
> > 
> 
> Hehe, I was about to ask the same question. :-)

No. it is not stored to fs.
Heh, the same question i asked my self. On linux-ima-user i found
this post (2009-07-23):
https://sourceforge.net/p/linux-ima/mailman/message/23152923/
---
When an inode entry is removed from dcache, the corresponding iint entry
is removed from the radix tree. Unmounting an fs will cause the inodes,
and by extension iint's, to be freed.  When the fs is remounted, any
file accessed will result in allocating a new iint structure with the
i_version set to 0.
---

The code seems to confirm it. So i assumed that IMA don't care if
i_version is stored to disk or not. And i_version is the only way
to notify IMA about inode changes.
Since IMA documentation explecitley set i_version as reqieremt, so this
option was provided as well.

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* Re: [PATCH v2 2/3] fs: ubifs: update i_version on inode changes
  2017-04-12  6:05       ` Oleksij Rempel
@ 2017-04-12  6:08         ` Christoph Hellwig
  2017-04-12  7:04           ` Oleksij Rempel
  0 siblings, 1 reply; 27+ messages in thread
From: Christoph Hellwig @ 2017-04-12  6:08 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Richard Weinberger, Christoph Hellwig, Oleksij Rempel, dedekind1,
	adrian.hunter, linux-mtd, linux-kernel, linux-fsdevel, kernel

On Wed, Apr 12, 2017 at 08:05:34AM +0200, Oleksij Rempel wrote:
> The code seems to confirm it. So i assumed that IMA don't care if
> i_version is stored to disk or not. And i_version is the only way
> to notify IMA about inode changes.
> Since IMA documentation explecitley set i_version as reqieremt, so this
> option was provided as well.

Maybe IMA doesn't care, but if you set MS_I_VERSION the fs does give
a guarantee.  Sp NAK on this patch as-is.

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

* Re: [PATCH v2 2/3] fs: ubifs: update i_version on inode changes
  2017-04-12  6:08         ` Christoph Hellwig
@ 2017-04-12  7:04           ` Oleksij Rempel
  2017-04-24 15:44             ` Richard Weinberger
  0 siblings, 1 reply; 27+ messages in thread
From: Oleksij Rempel @ 2017-04-12  7:04 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Richard Weinberger, Oleksij Rempel, dedekind1, adrian.hunter,
	linux-mtd, linux-kernel, linux-fsdevel, kernel

On Tue, Apr 11, 2017 at 11:08:47PM -0700, Christoph Hellwig wrote:
> On Wed, Apr 12, 2017 at 08:05:34AM +0200, Oleksij Rempel wrote:
> > The code seems to confirm it. So i assumed that IMA don't care if
> > i_version is stored to disk or not. And i_version is the only way
> > to notify IMA about inode changes.
> > Since IMA documentation explecitley set i_version as reqieremt, so this
> > option was provided as well.
> 
> Maybe IMA doesn't care, but if you set MS_I_VERSION the fs does give
> a guarantee.  Sp NAK on this patch as-is.

Ok, it was an expekted NACK :)
Suddenly right now i don't have good ide to solve it. IMA just won't to
know if some runtime changes was made to FS. Currently i can image
fallowing variants:
- rework IMA
- add MS_I_TEMP_VERSION and keep i_version using for it.
- add new variable for external use only. For example: ima_rt_i_version,
  or some thing like this.

Other ideas?

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* Re: [PATCH v2 3/3] fs: ubifs: set s_uuid in super block
  2017-04-12  5:48     ` Christoph Hellwig
@ 2017-04-12  7:15       ` Oleksij Rempel
  2017-04-24 15:47         ` Richard Weinberger
  0 siblings, 1 reply; 27+ messages in thread
From: Oleksij Rempel @ 2017-04-12  7:15 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Richard Weinberger, Oleksij Rempel, dedekind1, adrian.hunter,
	linux-mtd, linux-kernel, linux-fsdevel, Steffen Trumtrar

On Tue, Apr 11, 2017 at 10:48:28PM -0700, Christoph Hellwig wrote:
> On Tue, Apr 11, 2017 at 10:43:26PM +0200, Richard Weinberger wrote:
> > Artem, do you remember why UBIFS didn't set s_uuid in first place?
> 
> It's an extremely odd field - only a hand full of file systems set it
> (e.g. XFS doesn't, although according to Mimi IMA supports XFS), and
> it's never even used outside of the IMA/EVM code.
> 
> We really need a feature flag that this field is valid that IMA can
> check before adding more support for it.

It seems to be used by mm/cleancache.c
void __cleancache_init_shared_fs()

but this affects only ocfs2.

So, if some flag should be implemented, who should do it? :)
If me, what flag should be created?

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

* Re: [PATCH v2 2/3] fs: ubifs: update i_version on inode changes
  2017-04-12  7:04           ` Oleksij Rempel
@ 2017-04-24 15:44             ` Richard Weinberger
  0 siblings, 0 replies; 27+ messages in thread
From: Richard Weinberger @ 2017-04-24 15:44 UTC (permalink / raw)
  To: Oleksij Rempel, Christoph Hellwig
  Cc: dedekind1, adrian.hunter, linux-mtd, linux-kernel, linux-fsdevel, kernel

Oleksij,

Am 12.04.2017 um 09:04 schrieb Oleksij Rempel:
> On Tue, Apr 11, 2017 at 11:08:47PM -0700, Christoph Hellwig wrote:
>> On Wed, Apr 12, 2017 at 08:05:34AM +0200, Oleksij Rempel wrote:
>>> The code seems to confirm it. So i assumed that IMA don't care if
>>> i_version is stored to disk or not. And i_version is the only way
>>> to notify IMA about inode changes.
>>> Since IMA documentation explecitley set i_version as reqieremt, so this
>>> option was provided as well.
>>
>> Maybe IMA doesn't care, but if you set MS_I_VERSION the fs does give
>> a guarantee.  Sp NAK on this patch as-is.
> 
> Ok, it was an expekted NACK :)
> Suddenly right now i don't have good ide to solve it. IMA just won't to
> know if some runtime changes was made to FS. Currently i can image
> fallowing variants:
> - rework IMA

I assumed that you checked that option already. I IMA *really* needs i_version,
we can think of an solution. Adding new filesystem features should be done with
care.

> - add MS_I_TEMP_VERSION and keep i_version using for it.

You mean a non-persistent i_version like fat and exofs use internally?

> - add new variable for external use only. For example: ima_rt_i_version,
>   or some thing like this.

hch will hate this for very good reasons. :-)

> Other ideas?

What about making i_version persistent?
We still have some empty fields in UBIFS' inode data structure.
But first we have to be very sure that we need it.

Artem, do you remember why UBIFS does not store i_version?

Thanks,
//richard

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

* Re: [PATCH v2 3/3] fs: ubifs: set s_uuid in super block
  2017-04-12  7:15       ` Oleksij Rempel
@ 2017-04-24 15:47         ` Richard Weinberger
  2017-04-27 22:03           ` Richard Weinberger
  0 siblings, 1 reply; 27+ messages in thread
From: Richard Weinberger @ 2017-04-24 15:47 UTC (permalink / raw)
  To: Oleksij Rempel, Christoph Hellwig
  Cc: dedekind1, adrian.hunter, linux-mtd, linux-kernel, linux-fsdevel,
	Steffen Trumtrar

Oleksij,

Am 12.04.2017 um 09:15 schrieb Oleksij Rempel:
> On Tue, Apr 11, 2017 at 10:48:28PM -0700, Christoph Hellwig wrote:
>> On Tue, Apr 11, 2017 at 10:43:26PM +0200, Richard Weinberger wrote:
>>> Artem, do you remember why UBIFS didn't set s_uuid in first place?
>>
>> It's an extremely odd field - only a hand full of file systems set it
>> (e.g. XFS doesn't, although according to Mimi IMA supports XFS), and
>> it's never even used outside of the IMA/EVM code.
>>
>> We really need a feature flag that this field is valid that IMA can
>> check before adding more support for it.
> 
> It seems to be used by mm/cleancache.c
> void __cleancache_init_shared_fs()
> 
> but this affects only ocfs2.
> 
> So, if some flag should be implemented, who should do it? :)

I'll not do it for you. ;)

> If me, what flag should be created?

A super block flag that denotes that s_uuid is valid.

Thanks,
//richard

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

* Re: [PATCH v2 3/3] fs: ubifs: set s_uuid in super block
  2017-04-24 15:47         ` Richard Weinberger
@ 2017-04-27 22:03           ` Richard Weinberger
  2017-04-28  8:53             ` Amir Goldstein
  2017-05-02  7:19             ` Amir Goldstein
  0 siblings, 2 replies; 27+ messages in thread
From: Richard Weinberger @ 2017-04-27 22:03 UTC (permalink / raw)
  To: Oleksij Rempel
  Cc: Christoph Hellwig, dedekind1, adrian.hunter, linux-mtd,
	linux-kernel, linux-fsdevel, Steffen Trumtrar

Am 24.04.2017 um 17:47 schrieb Richard Weinberger:
>> So, if some flag should be implemented, who should do it? :)
> 
> I'll not do it for you. ;)

Please also see http://marc.info/?l=linux-fsdevel&m=149327990608749&w=2

Thanks,
//richard

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

* Re: [PATCH v2 3/3] fs: ubifs: set s_uuid in super block
  2017-04-27 22:03           ` Richard Weinberger
@ 2017-04-28  8:53             ` Amir Goldstein
  2017-05-02  5:30               ` Oleksij Rempel
  2017-05-02  7:19             ` Amir Goldstein
  1 sibling, 1 reply; 27+ messages in thread
From: Amir Goldstein @ 2017-04-28  8:53 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Oleksij Rempel, Christoph Hellwig, Artem Bityutskiy,
	Adrian Hunter, linux-mtd, linux-kernel, linux-fsdevel,
	Steffen Trumtrar

On Fri, Apr 28, 2017 at 1:03 AM, Richard Weinberger <richard@nod.at> wrote:
> Am 24.04.2017 um 17:47 schrieb Richard Weinberger:
>>> So, if some flag should be implemented, who should do it? :)
>>
>> I'll not do it for you. ;)
>
> Please also see http://marc.info/?l=linux-fsdevel&m=149327990608749&w=2
>

Perhaps you meant this:
https://marc.info/?l=linux-fsdevel&m=149328358909709&w=2

There does not seem to be much objections to adding the flag,
so hopefully, we can merge it for v.12 and filesystems and consumers
will pick it up whenever.

Amir.

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

* Re: [PATCH v2 3/3] fs: ubifs: set s_uuid in super block
  2017-04-28  8:53             ` Amir Goldstein
@ 2017-05-02  5:30               ` Oleksij Rempel
  0 siblings, 0 replies; 27+ messages in thread
From: Oleksij Rempel @ 2017-05-02  5:30 UTC (permalink / raw)
  To: Amir Goldstein, Richard Weinberger
  Cc: Christoph Hellwig, Artem Bityutskiy, Adrian Hunter, linux-mtd,
	linux-kernel, linux-fsdevel, Steffen Trumtrar

On 04/28/2017 10:53 AM, Amir Goldstein wrote:
> On Fri, Apr 28, 2017 at 1:03 AM, Richard Weinberger <richard@nod.at> wrote:
>> Am 24.04.2017 um 17:47 schrieb Richard Weinberger:
>>>> So, if some flag should be implemented, who should do it? :)
>>>
>>> I'll not do it for you. ;)
>>
>> Please also see http://marc.info/?l=linux-fsdevel&m=149327990608749&w=2
>>
>
> Perhaps you meant this:
> https://marc.info/?l=linux-fsdevel&m=149328358909709&w=2
>
> There does not seem to be much objections to adding the flag,
> so hopefully, we can merge it for v.12 and filesystems and consumers
> will pick it up whenever.

Ok, thanks.

Then i will need to wait untill your patches is merged and then resent 
updated patch to avoid merge race condition.

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

* Re: [PATCH v2 3/3] fs: ubifs: set s_uuid in super block
  2017-04-27 22:03           ` Richard Weinberger
  2017-04-28  8:53             ` Amir Goldstein
@ 2017-05-02  7:19             ` Amir Goldstein
  2017-05-02  7:37               ` Richard Weinberger
  1 sibling, 1 reply; 27+ messages in thread
From: Amir Goldstein @ 2017-05-02  7:19 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Oleksij Rempel, Christoph Hellwig, Artem Bityutskiy,
	Adrian Hunter, linux-mtd, linux-kernel, linux-fsdevel,
	Steffen Trumtrar

On Fri, Apr 28, 2017 at 1:03 AM, Richard Weinberger <richard@nod.at> wrote:
> Am 24.04.2017 um 17:47 schrieb Richard Weinberger:
>>> So, if some flag should be implemented, who should do it? :)
>>
>> I'll not do it for you. ;)
>
> Please also see http://marc.info/?l=linux-fsdevel&m=149327990608749&w=2
>

Richard,

Considering the facts that:
1. I proposed the said flag and Al didn't think it was needed [1]
2. ext4 already sets s_uuid without any flag for a long time now
3. A similar patch was queued for v4.12 to set s_uuid for xfs without any flag

I think it would be right to take Oleksij's patch as is.

FYI, my current work on 'constant inode numbers for overlayfs' requires that
underlying filesystem had set a non-zero s_uuid. Not sure if that matters for
ubifs+overlayfs users.

Amir.

[1] https://marc.info/?l=linux-unionfs&m=149352864527985&w=2

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

* Re: [PATCH v2 3/3] fs: ubifs: set s_uuid in super block
  2017-04-11 20:43   ` Richard Weinberger
  2017-04-12  5:48     ` Christoph Hellwig
@ 2017-05-02  7:23     ` Artem Bityutskiy
  1 sibling, 0 replies; 27+ messages in thread
From: Artem Bityutskiy @ 2017-05-02  7:23 UTC (permalink / raw)
  To: Richard Weinberger, Oleksij Rempel, adrian.hunter, linux-mtd,
	linux-kernel, linux-fsdevel
  Cc: Steffen Trumtrar

On Tue, 2017-04-11 at 22:43 +0200, Richard Weinberger wrote:
> Makes sense.
> 
> Artem, do you remember why UBIFS didn't set s_uuid in first place?

Just did not notice it I think.

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

* Re: [PATCH v2 3/3] fs: ubifs: set s_uuid in super block
  2017-05-02  7:19             ` Amir Goldstein
@ 2017-05-02  7:37               ` Richard Weinberger
  2017-05-09  4:13                 ` Oleksij Rempel
  0 siblings, 1 reply; 27+ messages in thread
From: Richard Weinberger @ 2017-05-02  7:37 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Oleksij Rempel, Christoph Hellwig, Artem Bityutskiy,
	Adrian Hunter, linux-mtd, linux-kernel, linux-fsdevel,
	Steffen Trumtrar

Amir,

Am 02.05.2017 um 09:19 schrieb Amir Goldstein:
> On Fri, Apr 28, 2017 at 1:03 AM, Richard Weinberger <richard@nod.at> wrote:
>> Am 24.04.2017 um 17:47 schrieb Richard Weinberger:
>>>> So, if some flag should be implemented, who should do it? :)
>>>
>>> I'll not do it for you. ;)
>>
>> Please also see http://marc.info/?l=linux-fsdevel&m=149327990608749&w=2
>>
> 
> Richard,
> 
> Considering the facts that:
> 1. I proposed the said flag and Al didn't think it was needed [1]
> 2. ext4 already sets s_uuid without any flag for a long time now
> 3. A similar patch was queued for v4.12 to set s_uuid for xfs without any flag
> 
> I think it would be right to take Oleksij's patch as is.
> 
> FYI, my current work on 'constant inode numbers for overlayfs' requires that
> underlying filesystem had set a non-zero s_uuid. Not sure if that matters for
> ubifs+overlayfs users.

If VFS maintainers are fine with that, I'll take it.
>From UBIFS' POV it does not matter much. :-)

Thanks
//richard

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

* Re: [PATCH v2 3/3] fs: ubifs: set s_uuid in super block
  2017-05-02  7:37               ` Richard Weinberger
@ 2017-05-09  4:13                 ` Oleksij Rempel
       [not found]                   ` <CAOQ4uxiEGJLSGS5rK8V8GRNvf9aWqbVG5odu2=nv73xTOmvfNQ@mail.gmail.com>
  0 siblings, 1 reply; 27+ messages in thread
From: Oleksij Rempel @ 2017-05-09  4:13 UTC (permalink / raw)
  To: Richard Weinberger, Amir Goldstein
  Cc: Christoph Hellwig, Artem Bityutskiy, Adrian Hunter, linux-mtd,
	linux-kernel, linux-fsdevel, Steffen Trumtrar



On 05/02/2017 09:37 AM, Richard Weinberger wrote:
> Amir,
>
> Am 02.05.2017 um 09:19 schrieb Amir Goldstein:
>> On Fri, Apr 28, 2017 at 1:03 AM, Richard Weinberger <richard@nod.at> wrote:
>>> Am 24.04.2017 um 17:47 schrieb Richard Weinberger:
>>>>> So, if some flag should be implemented, who should do it? :)
>>>>
>>>> I'll not do it for you. ;)
>>>
>>> Please also see http://marc.info/?l=linux-fsdevel&m=149327990608749&w=2
>>>
>>
>> Richard,
>>
>> Considering the facts that:
>> 1. I proposed the said flag and Al didn't think it was needed [1]
>> 2. ext4 already sets s_uuid without any flag for a long time now
>> 3. A similar patch was queued for v4.12 to set s_uuid for xfs without any flag
>>
>> I think it would be right to take Oleksij's patch as is.
>>
>> FYI, my current work on 'constant inode numbers for overlayfs' requires that
>> underlying filesystem had set a non-zero s_uuid. Not sure if that matters for
>> ubifs+overlayfs users.
>
> If VFS maintainers are fine with that, I'll take it.
> From UBIFS' POV it does not matter much. :-)

Ping to VFS maintainers?

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

* Re: [PATCH v2 3/3] fs: ubifs: set s_uuid in super block
       [not found]                   ` <CAOQ4uxiEGJLSGS5rK8V8GRNvf9aWqbVG5odu2=nv73xTOmvfNQ@mail.gmail.com>
@ 2017-05-09  5:52                     ` Oleksij Rempel
  2017-05-09  7:01                       ` Richard Weinberger
  0 siblings, 1 reply; 27+ messages in thread
From: Oleksij Rempel @ 2017-05-09  5:52 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Richard Weinberger, Christoph Hellwig, Artem Bityutskiy,
	Adrian Hunter, linux-mtd, linux-kernel, linux-fsdevel,
	Steffen Trumtrar



On 05/09/2017 07:37 AM, Amir Goldstein wrote:
>
>
> On Tue, May 9, 2017 at 7:13 AM, Oleksij Rempel <ore@pengutronix.de
> <mailto:ore@pengutronix.de>> wrote:
>
>
>
>     On 05/02/2017 09:37 AM, Richard Weinberger wrote:
>
>         Amir,
>
>         Am 02.05.2017 um 09:19 schrieb Amir Goldstein:
>
>             On Fri, Apr 28, 2017 at 1:03 AM, Richard Weinberger
>             <richard@nod.at <mailto:richard@nod.at>> wrote:
>
>                 Am 24.04.2017 um 17:47 schrieb Richard Weinberger:
>
>                         So, if some flag should be implemented, who
>                         should do it? :)
>
>
>                     I'll not do it for you. ;)
>
>
>                 Please also see
>                 http://marc.info/?l=linux-fsdevel&m=149327990608749&w=2
>                 <http://marc.info/?l=linux-fsdevel&m=149327990608749&w=2>
>
>
>             Richard,
>
>             Considering the facts that:
>             1. I proposed the said flag and Al didn't think it was
>             needed [1]
>             2. ext4 already sets s_uuid without any flag for a long time now
>             3. A similar patch was queued for v4.12 to set s_uuid for
>             xfs without any flag
>
>             I think it would be right to take Oleksij's patch as is.
>
>             FYI, my current work on 'constant inode numbers for
>             overlayfs' requires that
>             underlying filesystem had set a non-zero s_uuid. Not sure if
>             that matters for
>             ubifs+overlayfs users.
>
>
>         If VFS maintainers are fine with that, I'll take it.
>         From UBIFS' POV it does not matter much. :-)
>
>
>     Ping to VFS maintainers?
>
>
> What ping? Al made it clear that a flag is not needed.
> BTW, xfs s_uuid patch was merged to master.


I'm talking about ubifs patch.

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

* Re: [PATCH v2 3/3] fs: ubifs: set s_uuid in super block
  2017-05-09  5:52                     ` Oleksij Rempel
@ 2017-05-09  7:01                       ` Richard Weinberger
  2017-05-09  7:08                         ` Amir Goldstein
  0 siblings, 1 reply; 27+ messages in thread
From: Richard Weinberger @ 2017-05-09  7:01 UTC (permalink / raw)
  To: Oleksij Rempel, Amir Goldstein
  Cc: Christoph Hellwig, Artem Bityutskiy, Adrian Hunter, linux-mtd,
	linux-kernel, linux-fsdevel, Steffen Trumtrar

Oleksij,

Am 09.05.2017 um 07:52 schrieb Oleksij Rempel:
>>
>>         If VFS maintainers are fine with that, I'll take it.
>>         From UBIFS' POV it does not matter much. :-)
>>
>>
>>     Ping to VFS maintainers?
>>
>>
>> What ping? Al made it clear that a flag is not needed.
>> BTW, xfs s_uuid patch was merged to master.
> 
> 
> I'm talking about ubifs patch.

Then we can queue this patch for 4.13.
Please resend and make sure it addresses everything what was also
suggested for the xfs s_uuid patch.

Thanks,
//richard

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

* Re: [PATCH v2 3/3] fs: ubifs: set s_uuid in super block
  2017-05-09  7:01                       ` Richard Weinberger
@ 2017-05-09  7:08                         ` Amir Goldstein
  2017-05-09  7:35                           ` Oleksij Rempel
  2017-05-09  7:35                           ` Richard Weinberger
  0 siblings, 2 replies; 27+ messages in thread
From: Amir Goldstein @ 2017-05-09  7:08 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Oleksij Rempel, Christoph Hellwig, Artem Bityutskiy,
	Adrian Hunter, linux-mtd, linux-kernel, linux-fsdevel,
	Steffen Trumtrar

On Tue, May 9, 2017 at 10:01 AM, Richard Weinberger <richard@nod.at> wrote:
>
> Oleksij,
>
> Am 09.05.2017 um 07:52 schrieb Oleksij Rempel:
> >>
> >>         If VFS maintainers are fine with that, I'll take it.
> >>         From UBIFS' POV it does not matter much. :-)
> >>
> >>
> >>     Ping to VFS maintainers?
> >>
> >>
> >> What ping? Al made it clear that a flag is not needed.
> >> BTW, xfs s_uuid patch was merged to master.
> >
> >
> > I'm talking about ubifs patch.

Me too.

>
> Then we can queue this patch for 4.13.
> Please resend and make sure it addresses everything what was also
> suggested for the xfs s_uuid patch.
>

Just to be clear, the xfs s_uuid patch is just a memcpy,
no different from Oleksij's patch.

Thanks,
Amir.

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

* Re: [PATCH v2 3/3] fs: ubifs: set s_uuid in super block
  2017-05-09  7:08                         ` Amir Goldstein
@ 2017-05-09  7:35                           ` Oleksij Rempel
  2017-05-09  7:35                           ` Richard Weinberger
  1 sibling, 0 replies; 27+ messages in thread
From: Oleksij Rempel @ 2017-05-09  7:35 UTC (permalink / raw)
  To: Amir Goldstein, Richard Weinberger
  Cc: Christoph Hellwig, Artem Bityutskiy, Adrian Hunter, linux-mtd,
	linux-kernel, linux-fsdevel, Steffen Trumtrar



On 05/09/2017 09:08 AM, Amir Goldstein wrote:
> On Tue, May 9, 2017 at 10:01 AM, Richard Weinberger <richard@nod.at> wrote:
>>
>> Oleksij,
>>
>> Am 09.05.2017 um 07:52 schrieb Oleksij Rempel:
>>>>
>>>>         If VFS maintainers are fine with that, I'll take it.
>>>>         From UBIFS' POV it does not matter much. :-)
>>>>
>>>>
>>>>     Ping to VFS maintainers?
>>>>
>>>>
>>>> What ping? Al made it clear that a flag is not needed.
>>>> BTW, xfs s_uuid patch was merged to master.
>>>
>>>
>>> I'm talking about ubifs patch.
>
> Me too.

:) ok

>>
>> Then we can queue this patch for 4.13.
>> Please resend and make sure it addresses everything what was also
>> suggested for the xfs s_uuid patch.
>>
>
> Just to be clear, the xfs s_uuid patch is just a memcpy,
> no different from Oleksij's patch.

So, should i change something?

here is the patch:
https://patchwork.kernel.org/patch/9674817/

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

* Re: [PATCH v2 3/3] fs: ubifs: set s_uuid in super block
  2017-05-09  7:08                         ` Amir Goldstein
  2017-05-09  7:35                           ` Oleksij Rempel
@ 2017-05-09  7:35                           ` Richard Weinberger
  2017-05-09  7:50                             ` Amir Goldstein
  1 sibling, 1 reply; 27+ messages in thread
From: Richard Weinberger @ 2017-05-09  7:35 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Oleksij Rempel, Christoph Hellwig, Artem Bityutskiy,
	Adrian Hunter, linux-mtd, linux-kernel, linux-fsdevel,
	Steffen Trumtrar

Amir,

Am 09.05.2017 um 09:08 schrieb Amir Goldstein:
>> Then we can queue this patch for 4.13.
>> Please resend and make sure it addresses everything what was also
>> suggested for the xfs s_uuid patch.
>>
> 
> Just to be clear, the xfs s_uuid patch is just a memcpy,
> no different from Oleksij's patch.

Wasn't there a huge discussion about LE/BE/uniqueness and more details
on UUID that hurt my brain.

Thanks,
//richard

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

* Re: [PATCH v2 3/3] fs: ubifs: set s_uuid in super block
  2017-05-09  7:35                           ` Richard Weinberger
@ 2017-05-09  7:50                             ` Amir Goldstein
  0 siblings, 0 replies; 27+ messages in thread
From: Amir Goldstein @ 2017-05-09  7:50 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: Oleksij Rempel, Christoph Hellwig, Artem Bityutskiy,
	Adrian Hunter, linux-mtd, linux-kernel, linux-fsdevel,
	Steffen Trumtrar

On Tue, May 9, 2017 at 10:35 AM, Richard Weinberger <richard@nod.at> wrote:
> Amir,
>
> Am 09.05.2017 um 09:08 schrieb Amir Goldstein:
>>> Then we can queue this patch for 4.13.
>>> Please resend and make sure it addresses everything what was also
>>> suggested for the xfs s_uuid patch.
>>>
>>
>> Just to be clear, the xfs s_uuid patch is just a memcpy,
>> no different from Oleksij's patch.

See upstream commit
8f720d9 xfs: publish UUID in struct super_block


>
> Wasn't there a huge discussion about LE/BE/uniqueness and more details
> on UUID that hurt my brain.
>

LE/BE discussions are more about which variants of uuid helpers should be
created, among other things, for consumers of s_uuid to check that s_uuid
was filled by fs.
Converting s_uuid type to uuid_t or whatever is for the far future.

uniqueness of s_uuid does not exist with current filesystems,
so no reason whatsoever to act differently with ubifs.
Fixing uniqueness of s_uuid (if at all is needed) is a future VFS task.

Bottom line, for Oleksij's original patch:

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

If it's not too late for 4.12 that could be nice, because then
ubifs+overlayfs would gain a new feature (constant inode numbers)

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

end of thread, other threads:[~2017-05-09  7:50 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-11  9:50 [PATCH v2 0/3] make ubifs compatible with IMA and EVM Oleksij Rempel
2017-04-11  9:50 ` [PATCH v2 1/3] fs: ubifs: parse iversion mount option Oleksij Rempel
2017-04-11  9:50 ` [PATCH v2 2/3] fs: ubifs: update i_version on inode changes Oleksij Rempel
2017-04-11 16:05   ` Christoph Hellwig
2017-04-11 21:13     ` Richard Weinberger
2017-04-12  6:05       ` Oleksij Rempel
2017-04-12  6:08         ` Christoph Hellwig
2017-04-12  7:04           ` Oleksij Rempel
2017-04-24 15:44             ` Richard Weinberger
2017-04-11  9:50 ` [PATCH v2 3/3] fs: ubifs: set s_uuid in super block Oleksij Rempel
2017-04-11 20:43   ` Richard Weinberger
2017-04-12  5:48     ` Christoph Hellwig
2017-04-12  7:15       ` Oleksij Rempel
2017-04-24 15:47         ` Richard Weinberger
2017-04-27 22:03           ` Richard Weinberger
2017-04-28  8:53             ` Amir Goldstein
2017-05-02  5:30               ` Oleksij Rempel
2017-05-02  7:19             ` Amir Goldstein
2017-05-02  7:37               ` Richard Weinberger
2017-05-09  4:13                 ` Oleksij Rempel
     [not found]                   ` <CAOQ4uxiEGJLSGS5rK8V8GRNvf9aWqbVG5odu2=nv73xTOmvfNQ@mail.gmail.com>
2017-05-09  5:52                     ` Oleksij Rempel
2017-05-09  7:01                       ` Richard Weinberger
2017-05-09  7:08                         ` Amir Goldstein
2017-05-09  7:35                           ` Oleksij Rempel
2017-05-09  7:35                           ` Richard Weinberger
2017-05-09  7:50                             ` Amir Goldstein
2017-05-02  7:23     ` Artem Bityutskiy

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