linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] jffs2: make the overwritten xattr invisible after remount
@ 2018-12-09  6:21 Hou Tao
  2018-12-15 13:23 ` Hou Tao
  0 siblings, 1 reply; 3+ messages in thread
From: Hou Tao @ 2018-12-09  6:21 UTC (permalink / raw)
  To: dwmw2, linux-mtd; +Cc: richard.weinberger, boris.brezillon, linux-kernel

For xattr modification, we do not write a new jffs2_raw_xref with
delete marker into flash, so if a xattr is modified then removed,
and the old xref & xdatum are not erased by GC, after reboot or
remount, the new xattr xref will be dead but the old xattr xref
will be alive, and we will get the overwritten xattr instead of
non-existent error when reading the removed xattr.

Fix it by writing the deletion mark for xattr overwrite.

Fixes: 8a13695cbe4e ("[JFFS2][XATTR] rid unnecessary writing of delete marker.")
Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 fs/jffs2/xattr.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 49 insertions(+), 6 deletions(-)

diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c
index da3e18503c65..b2d6072f34af 100644
--- a/fs/jffs2/xattr.c
+++ b/fs/jffs2/xattr.c
@@ -573,6 +573,15 @@ static struct jffs2_xattr_ref *create_xattr_ref(struct jffs2_sb_info *c, struct
 	return ref; /* success */
 }
 
+static void move_xattr_ref_to_dead_list(struct jffs2_sb_info *c,
+		struct jffs2_xattr_ref *ref)
+{
+	spin_lock(&c->erase_completion_lock);
+	ref->next = c->xref_dead_list;
+	c->xref_dead_list = ref;
+	spin_unlock(&c->erase_completion_lock);
+}
+
 static void delete_xattr_ref(struct jffs2_sb_info *c, struct jffs2_xattr_ref *ref)
 {
 	/* must be called under down_write(xattr_sem) */
@@ -582,10 +591,7 @@ static void delete_xattr_ref(struct jffs2_sb_info *c, struct jffs2_xattr_ref *re
 	ref->xseqno |= XREF_DELETE_MARKER;
 	ref->ino = ref->ic->ino;
 	ref->xid = ref->xd->xid;
-	spin_lock(&c->erase_completion_lock);
-	ref->next = c->xref_dead_list;
-	c->xref_dead_list = ref;
-	spin_unlock(&c->erase_completion_lock);
+	move_xattr_ref_to_dead_list(c, ref);
 
 	dbg_xattr("xref(ino=%u, xid=%u, xseqno=%u) was removed.\n",
 		  ref->ino, ref->xid, ref->xseqno);
@@ -1090,6 +1096,40 @@ int do_jffs2_getxattr(struct inode *inode, int xprefix, const char *xname,
 	return rc;
 }
 
+static void do_jffs2_delete_xattr_ref(struct jffs2_sb_info *c,
+		struct jffs2_xattr_ref *ref)
+{
+	uint32_t request, length;
+	int err;
+	struct jffs2_xattr_datum *xd;
+
+	request = PAD(sizeof(struct jffs2_raw_xref));
+	err = jffs2_reserve_space(c, request, &length,
+			ALLOC_NORMAL, JFFS2_SUMMARY_XREF_SIZE);
+	down_write(&c->xattr_sem);
+	if (err) {
+		JFFS2_WARNING("jffs2_reserve_space()=%d, request=%u\n",
+				err, request);
+		delete_xattr_ref(c, ref);
+		up_write(&c->xattr_sem);
+		return;
+	}
+
+	xd = ref->xd;
+	ref->ino = ref->ic->ino;
+	ref->xid = xd->xid;
+	ref->xseqno |= XREF_DELETE_MARKER;
+	save_xattr_ref(c, ref);
+
+	move_xattr_ref_to_dead_list(c, ref);
+	dbg_xattr("xref(ino=%u, xid=%u, xseqno=%u) was removed.\n",
+		  ref->ino, ref->xid, ref->xseqno);
+	unrefer_xattr_datum(c, xd);
+
+	up_write(&c->xattr_sem);
+	jffs2_complete_reservation(c);
+}
+
 int do_jffs2_setxattr(struct inode *inode, int xprefix, const char *xname,
 		      const char *buffer, size_t size, int flags)
 {
@@ -1097,7 +1137,7 @@ int do_jffs2_setxattr(struct inode *inode, int xprefix, const char *xname,
 	struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb);
 	struct jffs2_inode_cache *ic = f->inocache;
 	struct jffs2_xattr_datum *xd;
-	struct jffs2_xattr_ref *ref, *newref, **pref;
+	struct jffs2_xattr_ref *ref, *newref, *oldref, **pref;
 	uint32_t length, request;
 	int rc;
 
@@ -1113,6 +1153,7 @@ int do_jffs2_setxattr(struct inode *inode, int xprefix, const char *xname,
 		return rc;
 	}
 
+	oldref = NULL;
 	/* Find existing xattr */
 	down_write(&c->xattr_sem);
  retry:
@@ -1196,11 +1237,13 @@ int do_jffs2_setxattr(struct inode *inode, int xprefix, const char *xname,
 		rc = PTR_ERR(newref);
 		unrefer_xattr_datum(c, xd);
 	} else if (ref) {
-		delete_xattr_ref(c, ref);
+		oldref = ref;
 	}
  out:
 	up_write(&c->xattr_sem);
 	jffs2_complete_reservation(c);
+	if (oldref)
+		do_jffs2_delete_xattr_ref(c, oldref);
 	return rc;
 }
 
-- 
2.16.2.dirty


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] jffs2: make the overwritten xattr invisible after remount
  2018-12-09  6:21 [PATCH] jffs2: make the overwritten xattr invisible after remount Hou Tao
@ 2018-12-15 13:23 ` Hou Tao
  2018-12-15 13:27   ` Richard Weinberger
  0 siblings, 1 reply; 3+ messages in thread
From: Hou Tao @ 2018-12-15 13:23 UTC (permalink / raw)
  To: dwmw2, linux-mtd; +Cc: richard.weinberger, boris.brezillon, linux-kernel

ping ?

On 2018/12/9 14:21, Hou Tao wrote:
> For xattr modification, we do not write a new jffs2_raw_xref with
> delete marker into flash, so if a xattr is modified then removed,
> and the old xref & xdatum are not erased by GC, after reboot or
> remount, the new xattr xref will be dead but the old xattr xref
> will be alive, and we will get the overwritten xattr instead of
> non-existent error when reading the removed xattr.
> 
> Fix it by writing the deletion mark for xattr overwrite.
> 
> Fixes: 8a13695cbe4e ("[JFFS2][XATTR] rid unnecessary writing of delete marker.")
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
>  fs/jffs2/xattr.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 49 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c
> index da3e18503c65..b2d6072f34af 100644
> --- a/fs/jffs2/xattr.c
> +++ b/fs/jffs2/xattr.c
> @@ -573,6 +573,15 @@ static struct jffs2_xattr_ref *create_xattr_ref(struct jffs2_sb_info *c, struct
>  	return ref; /* success */
>  }
>  
> +static void move_xattr_ref_to_dead_list(struct jffs2_sb_info *c,
> +		struct jffs2_xattr_ref *ref)
> +{
> +	spin_lock(&c->erase_completion_lock);
> +	ref->next = c->xref_dead_list;
> +	c->xref_dead_list = ref;
> +	spin_unlock(&c->erase_completion_lock);
> +}
> +
>  static void delete_xattr_ref(struct jffs2_sb_info *c, struct jffs2_xattr_ref *ref)
>  {
>  	/* must be called under down_write(xattr_sem) */
> @@ -582,10 +591,7 @@ static void delete_xattr_ref(struct jffs2_sb_info *c, struct jffs2_xattr_ref *re
>  	ref->xseqno |= XREF_DELETE_MARKER;
>  	ref->ino = ref->ic->ino;
>  	ref->xid = ref->xd->xid;
> -	spin_lock(&c->erase_completion_lock);
> -	ref->next = c->xref_dead_list;
> -	c->xref_dead_list = ref;
> -	spin_unlock(&c->erase_completion_lock);
> +	move_xattr_ref_to_dead_list(c, ref);
>  
>  	dbg_xattr("xref(ino=%u, xid=%u, xseqno=%u) was removed.\n",
>  		  ref->ino, ref->xid, ref->xseqno);
> @@ -1090,6 +1096,40 @@ int do_jffs2_getxattr(struct inode *inode, int xprefix, const char *xname,
>  	return rc;
>  }
>  
> +static void do_jffs2_delete_xattr_ref(struct jffs2_sb_info *c,
> +		struct jffs2_xattr_ref *ref)
> +{
> +	uint32_t request, length;
> +	int err;
> +	struct jffs2_xattr_datum *xd;
> +
> +	request = PAD(sizeof(struct jffs2_raw_xref));
> +	err = jffs2_reserve_space(c, request, &length,
> +			ALLOC_NORMAL, JFFS2_SUMMARY_XREF_SIZE);
> +	down_write(&c->xattr_sem);
> +	if (err) {
> +		JFFS2_WARNING("jffs2_reserve_space()=%d, request=%u\n",
> +				err, request);
> +		delete_xattr_ref(c, ref);
> +		up_write(&c->xattr_sem);
> +		return;
> +	}
> +
> +	xd = ref->xd;
> +	ref->ino = ref->ic->ino;
> +	ref->xid = xd->xid;
> +	ref->xseqno |= XREF_DELETE_MARKER;
> +	save_xattr_ref(c, ref);
> +
> +	move_xattr_ref_to_dead_list(c, ref);
> +	dbg_xattr("xref(ino=%u, xid=%u, xseqno=%u) was removed.\n",
> +		  ref->ino, ref->xid, ref->xseqno);
> +	unrefer_xattr_datum(c, xd);
> +
> +	up_write(&c->xattr_sem);
> +	jffs2_complete_reservation(c);
> +}
> +
>  int do_jffs2_setxattr(struct inode *inode, int xprefix, const char *xname,
>  		      const char *buffer, size_t size, int flags)
>  {
> @@ -1097,7 +1137,7 @@ int do_jffs2_setxattr(struct inode *inode, int xprefix, const char *xname,
>  	struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb);
>  	struct jffs2_inode_cache *ic = f->inocache;
>  	struct jffs2_xattr_datum *xd;
> -	struct jffs2_xattr_ref *ref, *newref, **pref;
> +	struct jffs2_xattr_ref *ref, *newref, *oldref, **pref;
>  	uint32_t length, request;
>  	int rc;
>  
> @@ -1113,6 +1153,7 @@ int do_jffs2_setxattr(struct inode *inode, int xprefix, const char *xname,
>  		return rc;
>  	}
>  
> +	oldref = NULL;
>  	/* Find existing xattr */
>  	down_write(&c->xattr_sem);
>   retry:
> @@ -1196,11 +1237,13 @@ int do_jffs2_setxattr(struct inode *inode, int xprefix, const char *xname,
>  		rc = PTR_ERR(newref);
>  		unrefer_xattr_datum(c, xd);
>  	} else if (ref) {
> -		delete_xattr_ref(c, ref);
> +		oldref = ref;
>  	}
>   out:
>  	up_write(&c->xattr_sem);
>  	jffs2_complete_reservation(c);
> +	if (oldref)
> +		do_jffs2_delete_xattr_ref(c, oldref);
>  	return rc;
>  }
>  
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] jffs2: make the overwritten xattr invisible after remount
  2018-12-15 13:23 ` Hou Tao
@ 2018-12-15 13:27   ` Richard Weinberger
  0 siblings, 0 replies; 3+ messages in thread
From: Richard Weinberger @ 2018-12-15 13:27 UTC (permalink / raw)
  To: houtao1; +Cc: David Woodhouse, linux-mtd, boris.brezillon, LKML

On Sat, Dec 15, 2018 at 2:23 PM Hou Tao <houtao1@huawei.com> wrote:
>
> ping ?

Sorry for the delay.
I'll try now to review all the pending jffs2 patches.

> On 2018/12/9 14:21, Hou Tao wrote:
> > For xattr modification, we do not write a new jffs2_raw_xref with
> > delete marker into flash, so if a xattr is modified then removed,
> > and the old xref & xdatum are not erased by GC, after reboot or
> > remount, the new xattr xref will be dead but the old xattr xref
> > will be alive, and we will get the overwritten xattr instead of
> > non-existent error when reading the removed xattr.
> >
> > Fix it by writing the deletion mark for xattr overwrite.
> >
> > Fixes: 8a13695cbe4e ("[JFFS2][XATTR] rid unnecessary writing of delete marker.")
> > Signed-off-by: Hou Tao <houtao1@huawei.com>
> > ---
> >  fs/jffs2/xattr.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 49 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/jffs2/xattr.c b/fs/jffs2/xattr.c
> > index da3e18503c65..b2d6072f34af 100644
> > --- a/fs/jffs2/xattr.c
> > +++ b/fs/jffs2/xattr.c
> > @@ -573,6 +573,15 @@ static struct jffs2_xattr_ref *create_xattr_ref(struct jffs2_sb_info *c, struct
> >       return ref; /* success */
> >  }
> >
> > +static void move_xattr_ref_to_dead_list(struct jffs2_sb_info *c,
> > +             struct jffs2_xattr_ref *ref)
> > +{
> > +     spin_lock(&c->erase_completion_lock);
> > +     ref->next = c->xref_dead_list;
> > +     c->xref_dead_list = ref;
> > +     spin_unlock(&c->erase_completion_lock);
> > +}
> > +
> >  static void delete_xattr_ref(struct jffs2_sb_info *c, struct jffs2_xattr_ref *ref)
> >  {
> >       /* must be called under down_write(xattr_sem) */
> > @@ -582,10 +591,7 @@ static void delete_xattr_ref(struct jffs2_sb_info *c, struct jffs2_xattr_ref *re
> >       ref->xseqno |= XREF_DELETE_MARKER;
> >       ref->ino = ref->ic->ino;
> >       ref->xid = ref->xd->xid;
> > -     spin_lock(&c->erase_completion_lock);
> > -     ref->next = c->xref_dead_list;
> > -     c->xref_dead_list = ref;
> > -     spin_unlock(&c->erase_completion_lock);
> > +     move_xattr_ref_to_dead_list(c, ref);
> >
> >       dbg_xattr("xref(ino=%u, xid=%u, xseqno=%u) was removed.\n",
> >                 ref->ino, ref->xid, ref->xseqno);
> > @@ -1090,6 +1096,40 @@ int do_jffs2_getxattr(struct inode *inode, int xprefix, const char *xname,
> >       return rc;
> >  }
> >
> > +static void do_jffs2_delete_xattr_ref(struct jffs2_sb_info *c,
> > +             struct jffs2_xattr_ref *ref)
> > +{
> > +     uint32_t request, length;
> > +     int err;
> > +     struct jffs2_xattr_datum *xd;
> > +
> > +     request = PAD(sizeof(struct jffs2_raw_xref));
> > +     err = jffs2_reserve_space(c, request, &length,
> > +                     ALLOC_NORMAL, JFFS2_SUMMARY_XREF_SIZE);
> > +     down_write(&c->xattr_sem);
> > +     if (err) {
> > +             JFFS2_WARNING("jffs2_reserve_space()=%d, request=%u\n",
> > +                             err, request);
> > +             delete_xattr_ref(c, ref);
> > +             up_write(&c->xattr_sem);
> > +             return;
> > +     }
> > +
> > +     xd = ref->xd;
> > +     ref->ino = ref->ic->ino;
> > +     ref->xid = xd->xid;
> > +     ref->xseqno |= XREF_DELETE_MARKER;
> > +     save_xattr_ref(c, ref);
> > +
> > +     move_xattr_ref_to_dead_list(c, ref);
> > +     dbg_xattr("xref(ino=%u, xid=%u, xseqno=%u) was removed.\n",
> > +               ref->ino, ref->xid, ref->xseqno);
> > +     unrefer_xattr_datum(c, xd);
> > +
> > +     up_write(&c->xattr_sem);
> > +     jffs2_complete_reservation(c);
> > +}
> > +
> >  int do_jffs2_setxattr(struct inode *inode, int xprefix, const char *xname,
> >                     const char *buffer, size_t size, int flags)
> >  {
> > @@ -1097,7 +1137,7 @@ int do_jffs2_setxattr(struct inode *inode, int xprefix, const char *xname,
> >       struct jffs2_sb_info *c = JFFS2_SB_INFO(inode->i_sb);
> >       struct jffs2_inode_cache *ic = f->inocache;
> >       struct jffs2_xattr_datum *xd;
> > -     struct jffs2_xattr_ref *ref, *newref, **pref;
> > +     struct jffs2_xattr_ref *ref, *newref, *oldref, **pref;
> >       uint32_t length, request;
> >       int rc;
> >
> > @@ -1113,6 +1153,7 @@ int do_jffs2_setxattr(struct inode *inode, int xprefix, const char *xname,
> >               return rc;
> >       }
> >
> > +     oldref = NULL;
> >       /* Find existing xattr */
> >       down_write(&c->xattr_sem);
> >   retry:
> > @@ -1196,11 +1237,13 @@ int do_jffs2_setxattr(struct inode *inode, int xprefix, const char *xname,
> >               rc = PTR_ERR(newref);
> >               unrefer_xattr_datum(c, xd);
> >       } else if (ref) {
> > -             delete_xattr_ref(c, ref);
> > +             oldref = ref;
> >       }
> >   out:
> >       up_write(&c->xattr_sem);
> >       jffs2_complete_reservation(c);
> > +     if (oldref)
> > +             do_jffs2_delete_xattr_ref(c, oldref);
> >       return rc;
> >  }
> >
> >
>


-- 
Thanks,
//richard

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-12-15 13:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-09  6:21 [PATCH] jffs2: make the overwritten xattr invisible after remount Hou Tao
2018-12-15 13:23 ` Hou Tao
2018-12-15 13:27   ` Richard Weinberger

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).