linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH v3 0/3] btrfs-progs: check: verify symlinks with append/immutable flags
  2018-06-07  6:55 [PATCH v3 0/3] btrfs-progs: check: verify symlinks with append/immutable flags Su Yue
@ 2018-06-07  6:54 ` Nikolay Borisov
  2018-06-07  9:10   ` Su Yue
  2018-06-07  6:55 ` [PATCH v3 1/3] btrfs-progs: check: check " Su Yue
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Nikolay Borisov @ 2018-06-07  6:54 UTC (permalink / raw)
  To: Su Yue, linux-btrfs



On  7.06.2018 09:55, Su Yue wrote:
> This patchset can be fetch from my github:
> https://github.com/Damenly/btrfs-progs/commits/odd_inode_flags
> It's based on kdave/devel whose HEAD is:
> commit 1c846faaf87fbb01e080c94098c02b1695ed86dd
> Author: Nikolay Borisov <nborisov@suse.com>
> Date:   Mon May 28 09:36:50 2018 +0300
> 
> 	btrfs-progs: Remove fs_info argument from write_ctree_super
> 	
> symlinks should never have append/immutable attributes.
> This patchset enables btrfs check to verify such corruption.
> 
> PATCH[1] is for original mode.
> PATCH[2] is for original mode.
> 
> PATCH[3] adds a test image.
> For further use, the directory is called bad-inode-flags.
> 
> #issue 133


So you areadding code to detect the problem but not to fix it. Is there
some technical reason why you didn't implement clearing those flags or
just didn't do it for this series? IMHO it will be good if the check +
repair code are part of the same series. Then you can also extend the
test case to cover the 2 functionalities?

> 
> ---
> Changelog:
> v3:
>   Use S_ISLNK instead of BTRFS_FT_SYMLINK. Thanks Misono.
>   Change the test image created by hand.
> v2:
>   Use "rec->errors |=" instead of "rec->errors =" in patch[1].
>   Define new error bit of invalid inode flags in lowmem mode.
>   Adjust print message in patch[2]. Thanks, Qu and Nikolay.
>   Rename test directory from odd-inode-flags to bad-inode-flags.
> 
> Su Yue (3):
>   btrfs-progs: check: check symlinks with append/immutable flags
>   btrfs-progs: check: lowmem: check symlinks with append/immutable flags
>   btrfs-progs: fsck-tests: add test case to check symlinks with bad
>     flags
> 
>  check/main.c                                     |   7 +++++++
>  check/mode-lowmem.c                              |  10 ++++++++++
>  check/mode-lowmem.h                              |   1 +
>  check/mode-original.h                            |   1 +
>  .../034-bad-inode-flags/default_case.img         | Bin 0 -> 4096 bytes
>  tests/fsck-tests/034-bad-inode-flags/test.sh     |  15 +++++++++++++++
>  6 files changed, 34 insertions(+)
>  create mode 100644 tests/fsck-tests/034-bad-inode-flags/default_case.img
>  create mode 100755 tests/fsck-tests/034-bad-inode-flags/test.sh
> 

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

* [PATCH v3 0/3] btrfs-progs: check: verify symlinks with append/immutable flags
@ 2018-06-07  6:55 Su Yue
  2018-06-07  6:54 ` Nikolay Borisov
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Su Yue @ 2018-06-07  6:55 UTC (permalink / raw)
  To: linux-btrfs; +Cc: suy.fnst

This patchset can be fetch from my github:
https://github.com/Damenly/btrfs-progs/commits/odd_inode_flags
It's based on kdave/devel whose HEAD is:
commit 1c846faaf87fbb01e080c94098c02b1695ed86dd
Author: Nikolay Borisov <nborisov@suse.com>
Date:   Mon May 28 09:36:50 2018 +0300

	btrfs-progs: Remove fs_info argument from write_ctree_super
	
symlinks should never have append/immutable attributes.
This patchset enables btrfs check to verify such corruption.

PATCH[1] is for original mode.
PATCH[2] is for original mode.

PATCH[3] adds a test image.
For further use, the directory is called bad-inode-flags.

#issue 133

---
Changelog:
v3:
  Use S_ISLNK instead of BTRFS_FT_SYMLINK. Thanks Misono.
  Change the test image created by hand.
v2:
  Use "rec->errors |=" instead of "rec->errors =" in patch[1].
  Define new error bit of invalid inode flags in lowmem mode.
  Adjust print message in patch[2]. Thanks, Qu and Nikolay.
  Rename test directory from odd-inode-flags to bad-inode-flags.

Su Yue (3):
  btrfs-progs: check: check symlinks with append/immutable flags
  btrfs-progs: check: lowmem: check symlinks with append/immutable flags
  btrfs-progs: fsck-tests: add test case to check symlinks with bad
    flags

 check/main.c                                     |   7 +++++++
 check/mode-lowmem.c                              |  10 ++++++++++
 check/mode-lowmem.h                              |   1 +
 check/mode-original.h                            |   1 +
 .../034-bad-inode-flags/default_case.img         | Bin 0 -> 4096 bytes
 tests/fsck-tests/034-bad-inode-flags/test.sh     |  15 +++++++++++++++
 6 files changed, 34 insertions(+)
 create mode 100644 tests/fsck-tests/034-bad-inode-flags/default_case.img
 create mode 100755 tests/fsck-tests/034-bad-inode-flags/test.sh

-- 
2.17.1




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

* [PATCH v3 1/3] btrfs-progs: check: check symlinks with append/immutable flags
  2018-06-07  6:55 [PATCH v3 0/3] btrfs-progs: check: verify symlinks with append/immutable flags Su Yue
  2018-06-07  6:54 ` Nikolay Borisov
@ 2018-06-07  6:55 ` Su Yue
  2018-06-07  6:55 ` [PATCH v3 2/3] btrfs-progs: check: lowmem: " Su Yue
  2018-06-07  6:55 ` [PATCH v3 3/3] btrfs-progs: fsck-tests: add test case to check symlinks with bad flags Su Yue
  3 siblings, 0 replies; 7+ messages in thread
From: Su Yue @ 2018-06-07  6:55 UTC (permalink / raw)
  To: linux-btrfs; +Cc: suy.fnst, David Sterba

Define new macro I_ERR_ODD_INODE_FLAGS to represents odd inode flags.

Symlinks should never have append/immutable flags.
While processing inodes, if found a symlink with append/immutable
flags, mark the inode record with I_ERR_ODD_INODE_FLAGS.

This is for original mode.

Issue: #133
Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 check/main.c          | 7 +++++++
 check/mode-original.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/check/main.c b/check/main.c
index 9e8a83f86ab0..3190b5d4f293 100644
--- a/check/main.c
+++ b/check/main.c
@@ -576,6 +576,8 @@ static void print_inode_error(struct btrfs_root *root, struct inode_record *rec)
 		fprintf(stderr, ", link count wrong");
 	if (errors & I_ERR_FILE_EXTENT_ORPHAN)
 		fprintf(stderr, ", orphan file extent");
+	if (errors & I_ERR_ODD_INODE_FLAGS)
+		fprintf(stderr, ", odd inode flags");
 	fprintf(stderr, "\n");
 	/* Print the orphan extents if needed */
 	if (errors & I_ERR_FILE_EXTENT_ORPHAN)
@@ -805,6 +807,7 @@ static int process_inode_item(struct extent_buffer *eb,
 {
 	struct inode_record *rec;
 	struct btrfs_inode_item *item;
+	u64 flags;
 
 	rec = active_node->current;
 	BUG_ON(rec->ino != key->objectid || rec->refs > 1);
@@ -822,6 +825,10 @@ static int process_inode_item(struct extent_buffer *eb,
 	rec->found_inode_item = 1;
 	if (rec->nlink == 0)
 		rec->errors |= I_ERR_NO_ORPHAN_ITEM;
+	flags = btrfs_inode_flags(eb, item);
+	if (S_ISLNK(rec->imode) &&
+	    flags & (BTRFS_INODE_IMMUTABLE | BTRFS_INODE_APPEND))
+		rec->errors |= I_ERR_ODD_INODE_FLAGS;
 	maybe_free_inode_rec(&active_node->inode_cache, rec);
 	return 0;
 }
diff --git a/check/mode-original.h b/check/mode-original.h
index 368de692fdd1..13cfa5b9e1b3 100644
--- a/check/mode-original.h
+++ b/check/mode-original.h
@@ -186,6 +186,7 @@ struct file_extent_hole {
 #define I_ERR_LINK_COUNT_WRONG		(1 << 13)
 #define I_ERR_FILE_EXTENT_ORPHAN	(1 << 14)
 #define I_ERR_FILE_EXTENT_TOO_LARGE	(1 << 15)
+#define I_ERR_ODD_INODE_FLAGS		(1 << 16)
 
 struct inode_record {
 	struct list_head backrefs;
-- 
2.17.1




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

* [PATCH v3 2/3] btrfs-progs: check: lowmem: check symlinks with append/immutable flags
  2018-06-07  6:55 [PATCH v3 0/3] btrfs-progs: check: verify symlinks with append/immutable flags Su Yue
  2018-06-07  6:54 ` Nikolay Borisov
  2018-06-07  6:55 ` [PATCH v3 1/3] btrfs-progs: check: check " Su Yue
@ 2018-06-07  6:55 ` Su Yue
  2018-06-07  6:55 ` [PATCH v3 3/3] btrfs-progs: fsck-tests: add test case to check symlinks with bad flags Su Yue
  3 siblings, 0 replies; 7+ messages in thread
From: Su Yue @ 2018-06-07  6:55 UTC (permalink / raw)
  To: linux-btrfs; +Cc: suy.fnst, David Sterba

Define new error bit INODE_FLAGS_ERROR to represents invalid inode
flags error.

Symlinks should never have append/immutable flags set.
While checking inodes, if found a symlink with append/immutable
flags, report and record the inode flags error.

This is for lowmem mode.

Issue: #133
Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 check/mode-lowmem.c | 10 ++++++++++
 check/mode-lowmem.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 989306f03c2a..66da45319053 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -2274,6 +2274,7 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path)
 	struct btrfs_key last_key;
 	u64 inode_id;
 	u32 mode;
+	u64 flags;
 	u64 nlink;
 	u64 nbytes;
 	u64 isize;
@@ -2307,10 +2308,19 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path)
 	isize = btrfs_inode_size(node, ii);
 	nbytes = btrfs_inode_nbytes(node, ii);
 	mode = btrfs_inode_mode(node, ii);
+	flags = btrfs_inode_flags(node, ii);
 	dir = imode_to_type(mode) == BTRFS_FT_DIR;
 	nlink = btrfs_inode_nlink(node, ii);
 	nodatasum = btrfs_inode_flags(node, ii) & BTRFS_INODE_NODATASUM;
 
+	if (S_ISLNK(mode) &&
+	    flags & (BTRFS_INODE_IMMUTABLE | BTRFS_INODE_APPEND)) {
+		err |= INODE_FLAGS_ERROR;
+		error(
+"symlinks must never have immutable/append flags set, root %llu inode item %llu flags %llu may be corrupted",
+		      root->objectid, inode_id, flags);
+	}
+
 	while (1) {
 		btrfs_item_key_to_cpu(path->nodes[0], &last_key, path->slots[0]);
 		ret = btrfs_next_item(root, path);
diff --git a/check/mode-lowmem.h b/check/mode-lowmem.h
index e7ba62e2413e..91f7b6b1db53 100644
--- a/check/mode-lowmem.h
+++ b/check/mode-lowmem.h
@@ -44,6 +44,7 @@
 #define DIR_COUNT_AGAIN         (1<<20) /* DIR isize should be recalculated */
 #define BG_ACCOUNTING_ERROR     (1<<21) /* Block group accounting error */
 #define FATAL_ERROR             (1<<22) /* Fatal bit for errno */
+#define INODE_FLAGS_ERROR	(1<<23) /* Invalid inode flags */
 
 /*
  * Error bit for low memory mode check.
-- 
2.17.1




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

* [PATCH v3 3/3] btrfs-progs: fsck-tests: add test case to check symlinks with bad flags
  2018-06-07  6:55 [PATCH v3 0/3] btrfs-progs: check: verify symlinks with append/immutable flags Su Yue
                   ` (2 preceding siblings ...)
  2018-06-07  6:55 ` [PATCH v3 2/3] btrfs-progs: check: lowmem: " Su Yue
@ 2018-06-07  6:55 ` Su Yue
  3 siblings, 0 replies; 7+ messages in thread
From: Su Yue @ 2018-06-07  6:55 UTC (permalink / raw)
  To: linux-btrfs; +Cc: suy.fnst, David Sterba

There are two bad symlinks in the test case.  One is with immutable
attribute.  Another one is with append attribute.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
Signed-off-by: David Sterba <dsterba@suse.com>
---
 .../034-bad-inode-flags/default_case.img         | Bin 0 -> 4096 bytes
 tests/fsck-tests/034-bad-inode-flags/test.sh     |  15 +++++++++++++++
 2 files changed, 15 insertions(+)
 create mode 100644 tests/fsck-tests/034-bad-inode-flags/default_case.img
 create mode 100755 tests/fsck-tests/034-bad-inode-flags/test.sh

diff --git a/tests/fsck-tests/034-bad-inode-flags/default_case.img b/tests/fsck-tests/034-bad-inode-flags/default_case.img
new file mode 100644
index 0000000000000000000000000000000000000000..beb5e02e37c0af2c838008964538a4e28d442049
GIT binary patch
literal 4096
zcmeH}X*d*I8^=dNrYyZ7MYihEGsIYWEfa>aju>OfG8EZoBD=9PJhmjI&|+WHn5>iC
zFr=(ewqY!d3fTrtn&&ahJb2$vy}jR`56*S2`~IK%T=$3n@0{ygC&@kC|I=9{5%Ax^
zy^}fi=8jm~D+=yK;7%1lg74{B&fSFA)5E(FD!#i57}*V^|LCq=y+8H|>=XDW69}ci
z>NLJR<Pb1IxX24(7t(8gyj^*#QZdH_gTa5iIg$Tu-AqE)t8Kld8O+BY4UY!|b8?B=
zN*(zP+|4cQ?g|~81H6F-%RD+6n9_Bp!?~j}DuLH058%U)g2tfQ(JXnz0paLIk%T04
z5tyi5%^`bI>k~jVQA@YzEKgC|J$PGe*5mxAM>P`lHVS3c<1N5IgSK;`T!q(#j%dD@
zdfgQ@C@y>*I$PMw6J;zCIwb0-eaKp(%&X6+7UI@sKknBs#NjBBY_1m46J6s^Jz}nk
z;R)2vp2PrL6lKjqdJ204(jbOy=cI%I0A9oMwxN`ji7M}QsCef(+ba;H`d&BX6Lp%%
zHPQG=(67>0N<_RdEjWN+F8xT4tY8-BW2Tot<ve7jcl>bwn==3I5#%FpNh*_9n$gOh
zdeBV54vIx|@n@HYojWf!^#RRN-464*TOW2U?q*OcoQx;oFSbVV)ZJje5S{8Z{HV8F
z+1K}J**{f$DI#NRHqNNm=PM$9;cb&s=)&77r}BY!j=#PuQI~IwYcvUUmZs;hY&MnJ
zPtaJ_6FSYP&<Zu#lGILnyCOL|3`j+>MUp>>tu`>8N!~nNk@Jk<&@@5r4^W{G*PhxQ
z)Kz#!a8Op#7+`Y_t{BwpkeKrIOtugwt<xvN>%zUnV9HpT!^K+|-Po1JX~D!RbYRsu
z(VjLpU@Cn<zwC_DJvAHF<wt3P#??ZKqj#HA1|K0<X<LcSDRY>t`HPH6`tmptJv7Xq
zy9|ueA@d>qn4Eys$e^70wTU6E0?DA+(Q?04o8oe9tltaf_Op=!!Xg6$i!VD>@p4wO
z(Z-BczfMFg5r=NvoeJin`32hCOy4j>-XTz)$yom>8p7&6=v_;czXORgW3D2#)@art
zF_xZnoP_Tr@-fSdtacv=yqv2xcCsJe2OZ-xRYBZLtdI6huMX(V>fAKZ5ZFmT2;4>T
zl6w+NxLUsp?KjDv<`w<QyNb+zukL}f$_U}Cvh|p05Sia}_<&O>9p(!9<#`+lQDTTH
zaV}oQd3vJJ<CYFj%zS*N$xnSeDH4Z0QFE2A{8t-HH9i<N>y$`5xq0h8U7KO5zVMiQ
z8>6N3LYyxnm9pv};~hdX{r3o-SD!~ce>9Ju{uG|+kW;y2jDc%>nKi#ql<NKJgE;YP
zA@Nq1q`<5OC5C3CNfAh!wfwPi^*#n?qCZ@h5dkSyp@WWoSQc~RA}1f}ZBTDOZ5d;+
z>gj#y*($Q=SGGRoEN$9gf*T7jDbXQdOmGNB>QI79TO(Sw*#-8JzEl>rru6O?U2Se^
zbx$J}Cums%Eoqbpg3T&~N|#zmZ8TMya81q&gyP)8aHRxJC1E4cXf?kP7sp`gL*SWv
zB>jpvWy=<)lJGNT)ss=p%I85TDIAK{$tELS2j?)6zXS*%RTlhsjd%RZTj#HuqXuz$
zgg+SH<3^V=9!4q1pLXsr{gZeV_)_qK^3!l?g<S(E*5CRZJjTp6(j4?KTRLqCgq!ER
zH#Jp-Yh_X=*tuQ4?AFE5qUj~)^o^im!Md`(H6m#lcmhs8kKoL;nz@LQKDK6m=6J72
zoI&TO85!w*Z%YPZ%I=ILERR)z*`%_RO5XeMJV&AWmkK@dmv6+Lcv=PJ9{$(*0mxkM
zuN=fQtiBBo@s_D~y0(fu1v(l4pfL^fN$N3kZqGm0^5I@+02spMSj<GhWG$BIMPKte
zMlmo6S#qgpQNFa%ZFLhC_CT|+wVUE`helvMUX198spNMcRJ^8wenduE@wuKV4HUC1
zoNivVC(Ia4lw|6lGAh2DG>|t$5~0mC=;h1X-}txd@J(~^Oonm3@3z2KkGz!JXl%h)
z$n!9;_HB04HBVjbhTZ3mXwvR1#6xA&<Q13YKj&5?!BSdbaMcU->IZqtqzAb|Paxoz
zP^knpDP0uR^$J%(W=zaXr=*aDm3XelW_G~z13C4&?M-vr&kAC4C9PK(tJhX2ogaIH
z4ymQy&bt5klB`pp!;|@eWL^AZgr-K;6a@GaGSc&wj}l2sb1){m+nvdd`C#Qf6S%nu
zmTX@_&a3kgx9Wdth|=nbMbfU1fbHZzBWWVRY{DkShVmPmGgk_tx9dp@*bWtppRwD*
zasH0(Xzv+3<_Q%PN($w)Fp`rQ3qK%`N9BEaV4)e>#c@!s3Q;KnxiEfRRmL8;YU!G=
z?f&E)hx=$|;B3royJTQOErEhAY4aODihn_4g6@voD#?&-wYIPKz*gpDVj>mrWaPK%
zB<-`G8;xfDk`Brk*sA*IZfI1#Za3v|vRy{uq{8y?dtc(8Z0syaI=vnG7q6C=^rS*^
znOa268Agzq_LMJs%=ha^*aUk8>%?IiuHLg_s=w;6u7yzNdVujcH!!%1DzGNoY%A35
zdLyg{L~JGHZ3zf9nZE4VLinCdMSL?sbN~JYXDB_l>d~<C)Hc*LjUIm-ktb_?y73)3
zp6(Xl&HySW;~lO|b(Xdu#<QQn0u2obnhTe=`iXe~VQ+}rp&Lbs-yO5x7mlIoq!73`
zwOTv*T>F*Re<GE+Gnb^K{@2W2C1O6jTf(@wdG+zofJMWi;~ZN@)aK<DERy4jvs}mj
PY1%LE6WAy44<zt6YgYym

literal 0
HcmV?d00001

diff --git a/tests/fsck-tests/034-bad-inode-flags/test.sh b/tests/fsck-tests/034-bad-inode-flags/test.sh
new file mode 100755
index 000000000000..4bdc2bf0376c
--- /dev/null
+++ b/tests/fsck-tests/034-bad-inode-flags/test.sh
@@ -0,0 +1,15 @@
+#!/bin/bash
+# In order to confirm that 'btrfs check' supports checking symlinks
+# with immutable/append attributes that are not possible to set by standard
+# syscall or ioctl so they're handled as corruption
+
+source "$TEST_TOP/common"
+
+check_prereq btrfs
+
+check_image() {
+	run_mustfail "check should report errors about inode flags" \
+        $SUDO_HELPER "$TOP/btrfs" check "$1"
+}
+
+check_all_images
-- 
2.17.1




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

* Re: [PATCH v3 0/3] btrfs-progs: check: verify symlinks with append/immutable flags
  2018-06-07  6:54 ` Nikolay Borisov
@ 2018-06-07  9:10   ` Su Yue
  2018-06-07 11:16     ` Nikolay Borisov
  0 siblings, 1 reply; 7+ messages in thread
From: Su Yue @ 2018-06-07  9:10 UTC (permalink / raw)
  To: Nikolay Borisov, linux-btrfs



On 06/07/2018 02:54 PM, Nikolay Borisov wrote:
> 
> 
> On  7.06.2018 09:55, Su Yue wrote:
>> This patchset can be fetch from my github:
>> https://github.com/Damenly/btrfs-progs/commits/odd_inode_flags
>> It's based on kdave/devel whose HEAD is:
>> commit 1c846faaf87fbb01e080c94098c02b1695ed86dd
>> Author: Nikolay Borisov <nborisov@suse.com>
>> Date:   Mon May 28 09:36:50 2018 +0300
>>
>> 	btrfs-progs: Remove fs_info argument from write_ctree_super
>> 	
>> symlinks should never have append/immutable attributes.
>> This patchset enables btrfs check to verify such corruption.
>>
>> PATCH[1] is for original mode.
>> PATCH[2] is for original mode.
>>
>> PATCH[3] adds a test image.
>> For further use, the directory is called bad-inode-flags.
>>
>> #issue 133
> 
> 
> So you areadding code to detect the problem but not to fix it. Is there
> some technical reason why you didn't implement clearing those flags or
> just didn't do it for this series? IMHO it will be good if the check +

TBH I just didn't plan to do it while writing the patches.
If the functionality of repair is necessary, I'm willing to
do it.

However, it seems I have to complete work about repair mismatched file 
type first. So we can verify whether a bad inode is a symlink with 
append/immutable attributes or a normal file set with BTRFS_FT_SYMLINK.

I'm fine that v2 patchset is to be replaced then support repair late
or to be reverted.

Thanks,
Su

> repair code are part of the same series. Then you can also extend the
> test case to cover the 2 functionalities?
> 
>>
>> ---
>> Changelog:
>> v3:
>>    Use S_ISLNK instead of BTRFS_FT_SYMLINK. Thanks Misono.
>>    Change the test image created by hand.
>> v2:
>>    Use "rec->errors |=" instead of "rec->errors =" in patch[1].
>>    Define new error bit of invalid inode flags in lowmem mode.
>>    Adjust print message in patch[2]. Thanks, Qu and Nikolay.
>>    Rename test directory from odd-inode-flags to bad-inode-flags.
>>
>> Su Yue (3):
>>    btrfs-progs: check: check symlinks with append/immutable flags
>>    btrfs-progs: check: lowmem: check symlinks with append/immutable flags
>>    btrfs-progs: fsck-tests: add test case to check symlinks with bad
>>      flags
>>
>>   check/main.c                                     |   7 +++++++
>>   check/mode-lowmem.c                              |  10 ++++++++++
>>   check/mode-lowmem.h                              |   1 +
>>   check/mode-original.h                            |   1 +
>>   .../034-bad-inode-flags/default_case.img         | Bin 0 -> 4096 bytes
>>   tests/fsck-tests/034-bad-inode-flags/test.sh     |  15 +++++++++++++++
>>   6 files changed, 34 insertions(+)
>>   create mode 100644 tests/fsck-tests/034-bad-inode-flags/default_case.img
>>   create mode 100755 tests/fsck-tests/034-bad-inode-flags/test.sh
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" 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	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 0/3] btrfs-progs: check: verify symlinks with append/immutable flags
  2018-06-07  9:10   ` Su Yue
@ 2018-06-07 11:16     ` Nikolay Borisov
  0 siblings, 0 replies; 7+ messages in thread
From: Nikolay Borisov @ 2018-06-07 11:16 UTC (permalink / raw)
  To: Su Yue, linux-btrfs



On  7.06.2018 12:10, Su Yue wrote:
> 
> 
> On 06/07/2018 02:54 PM, Nikolay Borisov wrote:
>>
>>
>> On  7.06.2018 09:55, Su Yue wrote:
>>> This patchset can be fetch from my github:
>>> https://github.com/Damenly/btrfs-progs/commits/odd_inode_flags
>>> It's based on kdave/devel whose HEAD is:
>>> commit 1c846faaf87fbb01e080c94098c02b1695ed86dd
>>> Author: Nikolay Borisov <nborisov@suse.com>
>>> Date:   Mon May 28 09:36:50 2018 +0300
>>>
>>>     btrfs-progs: Remove fs_info argument from write_ctree_super
>>>     
>>> symlinks should never have append/immutable attributes.
>>> This patchset enables btrfs check to verify such corruption.
>>>
>>> PATCH[1] is for original mode.
>>> PATCH[2] is for original mode.
>>>
>>> PATCH[3] adds a test image.
>>> For further use, the directory is called bad-inode-flags.
>>>
>>> #issue 133
>>
>>
>> So you areadding code to detect the problem but not to fix it. Is there
>> some technical reason why you didn't implement clearing those flags or
>> just didn't do it for this series? IMHO it will be good if the check +
> 
> TBH I just didn't plan to do it while writing the patches.
> If the functionality of repair is necessary, I'm willing to
> do it.

That's fine, however in case this code detects such a symlink what's the
user supposed to do - they can just delete the symlink because it's
immutable/append-only. SO they will know they have such symlinks but
won't be able to repair them.

> 
> However, it seems I have to complete work about repair mismatched file
> type first. So we can verify whether a bad inode is a symlink with
> append/immutable attributes or a normal file set with BTRFS_FT_SYMLINK.
> 
> I'm fine that v2 patchset is to be replaced then support repair late
> or to be reverted.
> 
> Thanks,
> Su
> 
>> repair code are part of the same series. Then you can also extend the
>> test case to cover the 2 functionalities?
>>
>>>
>>> ---
>>> Changelog:
>>> v3:
>>>    Use S_ISLNK instead of BTRFS_FT_SYMLINK. Thanks Misono.
>>>    Change the test image created by hand.
>>> v2:
>>>    Use "rec->errors |=" instead of "rec->errors =" in patch[1].
>>>    Define new error bit of invalid inode flags in lowmem mode.
>>>    Adjust print message in patch[2]. Thanks, Qu and Nikolay.
>>>    Rename test directory from odd-inode-flags to bad-inode-flags.
>>>
>>> Su Yue (3):
>>>    btrfs-progs: check: check symlinks with append/immutable flags
>>>    btrfs-progs: check: lowmem: check symlinks with append/immutable
>>> flags
>>>    btrfs-progs: fsck-tests: add test case to check symlinks with bad
>>>      flags
>>>
>>>   check/main.c                                     |   7 +++++++
>>>   check/mode-lowmem.c                              |  10 ++++++++++
>>>   check/mode-lowmem.h                              |   1 +
>>>   check/mode-original.h                            |   1 +
>>>   .../034-bad-inode-flags/default_case.img         | Bin 0 -> 4096 bytes
>>>   tests/fsck-tests/034-bad-inode-flags/test.sh     |  15 +++++++++++++++
>>>   6 files changed, 34 insertions(+)
>>>   create mode 100644
>>> tests/fsck-tests/034-bad-inode-flags/default_case.img
>>>   create mode 100755 tests/fsck-tests/034-bad-inode-flags/test.sh
>>>
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" 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	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-06-07 11:16 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-07  6:55 [PATCH v3 0/3] btrfs-progs: check: verify symlinks with append/immutable flags Su Yue
2018-06-07  6:54 ` Nikolay Borisov
2018-06-07  9:10   ` Su Yue
2018-06-07 11:16     ` Nikolay Borisov
2018-06-07  6:55 ` [PATCH v3 1/3] btrfs-progs: check: check " Su Yue
2018-06-07  6:55 ` [PATCH v3 2/3] btrfs-progs: check: lowmem: " Su Yue
2018-06-07  6:55 ` [PATCH v3 3/3] btrfs-progs: fsck-tests: add test case to check symlinks with bad flags Su Yue

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