* [PATCH 0/3] btrfs-progs: check: verify symlinks with append/immutable flags @ 2018-05-14 10:29 Su Yue 2018-05-14 10:29 ` [PATCH 1/3] btrfs-progs: check: check " Su Yue ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Su Yue @ 2018-05-14 10:29 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 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 odd-inode-flags. #issue 133 Su Yue (3): btrfs-progs: check: check symlinks with append/immutable flags btrfs-progs: lowmem: check symlinks with append/immutable flags btrfs-progs: fsck-tests: add test case to check symlinks with odd flags check/main.c | 7 +++++++ check/mode-lowmem.c | 10 ++++++++++ check/mode-original.h | 1 + .../034-odd-inode-flags/default_case.img | Bin 0 -> 4096 bytes tests/fsck-tests/034-odd-inode-flags/test.sh | 15 +++++++++++++++ 5 files changed, 33 insertions(+) create mode 100644 tests/fsck-tests/034-odd-inode-flags/default_case.img create mode 100755 tests/fsck-tests/034-odd-inode-flags/test.sh -- 2.17.0 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] btrfs-progs: check: check symlinks with append/immutable flags 2018-05-14 10:29 [PATCH 0/3] btrfs-progs: check: verify symlinks with append/immutable flags Su Yue @ 2018-05-14 10:29 ` Su Yue 2018-05-14 11:18 ` Nikolay Borisov 2018-05-14 11:22 ` Qu Wenruo 2018-05-14 10:29 ` [PATCH 2/3] btrfs-progs: lowmem: " Su Yue 2018-05-14 10:29 ` [PATCH 3/3] btrfs-progs: fsck-tests: add test case to check symlinks with odd flags Su Yue 2 siblings, 2 replies; 11+ messages in thread From: Su Yue @ 2018-05-14 10:29 UTC (permalink / raw) To: linux-btrfs; +Cc: suy.fnst 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. Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.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 68da994f7ae0..f5f52c406046 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 (rec->imode & BTRFS_FT_SYMLINK && + 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.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] btrfs-progs: check: check symlinks with append/immutable flags 2018-05-14 10:29 ` [PATCH 1/3] btrfs-progs: check: check " Su Yue @ 2018-05-14 11:18 ` Nikolay Borisov 2018-05-14 12:12 ` Su Yue 2018-05-14 11:22 ` Qu Wenruo 1 sibling, 1 reply; 11+ messages in thread From: Nikolay Borisov @ 2018-05-14 11:18 UTC (permalink / raw) To: Su Yue, linux-btrfs On 14.05.2018 13:29, Su Yue wrote: > 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. > > Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.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 68da994f7ae0..f5f52c406046 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 (rec->imode & BTRFS_FT_SYMLINK && > + 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) I don't like the name of the flag. How about: I_ERR_INVALID_SYMLINK_FLAGS > > struct inode_record { > struct list_head backrefs; > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] btrfs-progs: check: check symlinks with append/immutable flags 2018-05-14 11:18 ` Nikolay Borisov @ 2018-05-14 12:12 ` Su Yue 0 siblings, 0 replies; 11+ messages in thread From: Su Yue @ 2018-05-14 12:12 UTC (permalink / raw) To: Nikolay Borisov, Su Yue, linux-btrfs [-- Attachment #1: Type: text/plain, Size: 2914 bytes --] On 2018/5/14 7:18 PM, Nikolay Borisov wrote: > > > On 14.05.2018 13:29, Su Yue wrote: >> 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. >> >> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.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 68da994f7ae0..f5f52c406046 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 (rec->imode & BTRFS_FT_SYMLINK && >> + 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) > > I don't like the name of the flag. How about: > > I_ERR_INVALID_SYMLINK_FLAGS > Before, original mode doesn't check inode flags. So I'd rather to keep INODE_FLAGS for further use. As for "ODD" part, it is just inherited from name style of above lines. I don't like "ODD" too much either. Anyway, thanks. Su >> >> struct inode_record { >> struct list_head backrefs; >> > -- > 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 > [-- Attachment #2: pEpkey.asc --] [-- Type: application/pgp-keys, Size: 1787 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] btrfs-progs: check: check symlinks with append/immutable flags 2018-05-14 10:29 ` [PATCH 1/3] btrfs-progs: check: check " Su Yue 2018-05-14 11:18 ` Nikolay Borisov @ 2018-05-14 11:22 ` Qu Wenruo 2018-05-14 11:52 ` Su Yue 1 sibling, 1 reply; 11+ messages in thread From: Qu Wenruo @ 2018-05-14 11:22 UTC (permalink / raw) To: Su Yue, linux-btrfs [-- Attachment #1.1: Type: text/plain, Size: 2333 bytes --] On 2018年05月14日 18:29, Su Yue wrote: > 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. > > Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.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 68da994f7ae0..f5f52c406046 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 (rec->imode & BTRFS_FT_SYMLINK && > + flags & (BTRFS_INODE_IMMUTABLE | BTRFS_INODE_APPEND)) > + rec->errors = I_ERR_ODD_INODE_FLAGS; Shouldn't it be "|=" instead of "=" ? Thanks, Qu > 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; > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] btrfs-progs: check: check symlinks with append/immutable flags 2018-05-14 11:22 ` Qu Wenruo @ 2018-05-14 11:52 ` Su Yue 0 siblings, 0 replies; 11+ messages in thread From: Su Yue @ 2018-05-14 11:52 UTC (permalink / raw) To: Qu Wenruo, Su Yue, linux-btrfs [-- Attachment #1: Type: text/plain, Size: 2491 bytes --] On 2018/5/14 7:22 PM, Qu Wenruo wrote: > > > On 2018年05月14日 18:29, Su Yue wrote: >> 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. >> >> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.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 68da994f7ae0..f5f52c406046 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 (rec->imode & BTRFS_FT_SYMLINK && >> + flags & (BTRFS_INODE_IMMUTABLE | BTRFS_INODE_APPEND)) >> + rec->errors = I_ERR_ODD_INODE_FLAGS; > > Shouldn't it be "|=" instead of "=" ? > > Thanks, > Qu Oh... Should be "|=". Thanks, Su > >> 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; >> > [-- Attachment #2: pEpkey.asc --] [-- Type: application/pgp-keys, Size: 1787 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] btrfs-progs: lowmem: check symlinks with append/immutable flags 2018-05-14 10:29 [PATCH 0/3] btrfs-progs: check: verify symlinks with append/immutable flags Su Yue 2018-05-14 10:29 ` [PATCH 1/3] btrfs-progs: check: check " Su Yue @ 2018-05-14 10:29 ` Su Yue 2018-05-14 11:17 ` Nikolay Borisov 2018-05-14 11:27 ` Qu Wenruo 2018-05-14 10:29 ` [PATCH 3/3] btrfs-progs: fsck-tests: add test case to check symlinks with odd flags Su Yue 2 siblings, 2 replies; 11+ messages in thread From: Su Yue @ 2018-05-14 10:29 UTC (permalink / raw) To: linux-btrfs; +Cc: suy.fnst Symlinks should never have append/immutable flags. While checking inodes, if found a symlink with append/immutable flags, report and record the fatal error. This is for lowmem mode. Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> --- check/mode-lowmem.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c index 9890180d1d3c..61c4e7f23d47 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 (mode & BTRFS_FT_SYMLINK && + (flags & (BTRFS_INODE_IMMUTABLE | BTRFS_INODE_APPEND))) { + err = FATAL_ERROR; + error( +"symlinks shall never be with immutable/append, 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); -- 2.17.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] btrfs-progs: lowmem: check symlinks with append/immutable flags 2018-05-14 10:29 ` [PATCH 2/3] btrfs-progs: lowmem: " Su Yue @ 2018-05-14 11:17 ` Nikolay Borisov 2018-05-14 12:04 ` Su Yue 2018-05-14 11:27 ` Qu Wenruo 1 sibling, 1 reply; 11+ messages in thread From: Nikolay Borisov @ 2018-05-14 11:17 UTC (permalink / raw) To: Su Yue, linux-btrfs On 14.05.2018 13:29, Su Yue wrote: > Symlinks should never have append/immutable flags. > While checking inodes, if found a symlink with append/immutable > flags, report and record the fatal error. > > This is for lowmem mode. > > Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> > --- > check/mode-lowmem.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c > index 9890180d1d3c..61c4e7f23d47 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 (mode & BTRFS_FT_SYMLINK && > + (flags & (BTRFS_INODE_IMMUTABLE | BTRFS_INODE_APPEND))) { > + err = FATAL_ERROR; > + error( > +"symlinks shall never be with immutable/append, root %llu inode item [%llu] flags %llu may be corrupted", How about: "symlinks must never have immutable/append flags set, root %llu ino %llu flag %llu may be corrupted". I think printing the ino number should suffice, no need for the "fancy" [] around the inode number. > + 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); > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] btrfs-progs: lowmem: check symlinks with append/immutable flags 2018-05-14 11:17 ` Nikolay Borisov @ 2018-05-14 12:04 ` Su Yue 0 siblings, 0 replies; 11+ messages in thread From: Su Yue @ 2018-05-14 12:04 UTC (permalink / raw) To: nborisov; +Cc: Su Yue, linux-btrfs Indeed better. Will do it in V2. Thanks, Su On Mon, May 14, 2018 at 7:19 PM Nikolay Borisov <nborisov@suse.com> wrote: > On 14.05.2018 13:29, Su Yue wrote: > > Symlinks should never have append/immutable flags. > > While checking inodes, if found a symlink with append/immutable > > flags, report and record the fatal error. > > > > This is for lowmem mode. > > > > Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> > > --- > > check/mode-lowmem.c | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c > > index 9890180d1d3c..61c4e7f23d47 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 (mode & BTRFS_FT_SYMLINK && > > + (flags & (BTRFS_INODE_IMMUTABLE | BTRFS_INODE_APPEND))) { > > + err = FATAL_ERROR; > > + error( > > +"symlinks shall never be with immutable/append, root %llu inode item [%llu] flags %llu may be corrupted", > How about: > "symlinks must never have immutable/append flags set, root %llu ino %llu flag %llu may be corrupted". > I think printing the ino number should suffice, no need for the "fancy" [] around the inode number. > > + 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); > > > -- > 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] 11+ messages in thread
* Re: [PATCH 2/3] btrfs-progs: lowmem: check symlinks with append/immutable flags 2018-05-14 10:29 ` [PATCH 2/3] btrfs-progs: lowmem: " Su Yue 2018-05-14 11:17 ` Nikolay Borisov @ 2018-05-14 11:27 ` Qu Wenruo 1 sibling, 0 replies; 11+ messages in thread From: Qu Wenruo @ 2018-05-14 11:27 UTC (permalink / raw) To: Su Yue, linux-btrfs [-- Attachment #1.1: Type: text/plain, Size: 1963 bytes --] On 2018年05月14日 18:29, Su Yue wrote: > Symlinks should never have append/immutable flags. > While checking inodes, if found a symlink with append/immutable > flags, report and record the fatal error. > > This is for lowmem mode. > > Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> > --- > check/mode-lowmem.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c > index 9890180d1d3c..61c4e7f23d47 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 (mode & BTRFS_FT_SYMLINK && > + (flags & (BTRFS_INODE_IMMUTABLE | BTRFS_INODE_APPEND))) { > + err = FATAL_ERROR; Where is the FATAL_ERROR defined? I can't find it anyway on v4.16 branch. And I assume that FATAL_ERROR is defined to abort check, if so, in that case the corruption is not that serious, just some unexpected behavior, not a fatal error. If not, just ignore this comment. Thanks, Qu > + error( > +"symlinks shall never be with immutable/append, 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); > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] btrfs-progs: fsck-tests: add test case to check symlinks with odd flags 2018-05-14 10:29 [PATCH 0/3] btrfs-progs: check: verify symlinks with append/immutable flags Su Yue 2018-05-14 10:29 ` [PATCH 1/3] btrfs-progs: check: check " Su Yue 2018-05-14 10:29 ` [PATCH 2/3] btrfs-progs: lowmem: " Su Yue @ 2018-05-14 10:29 ` Su Yue 2 siblings, 0 replies; 11+ messages in thread From: Su Yue @ 2018-05-14 10:29 UTC (permalink / raw) To: linux-btrfs; +Cc: suy.fnst 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> --- .../034-odd-inode-flags/default_case.img | Bin 0 -> 4096 bytes tests/fsck-tests/034-odd-inode-flags/test.sh | 15 +++++++++++++++ 2 files changed, 15 insertions(+) create mode 100644 tests/fsck-tests/034-odd-inode-flags/default_case.img create mode 100755 tests/fsck-tests/034-odd-inode-flags/test.sh diff --git a/tests/fsck-tests/034-odd-inode-flags/default_case.img b/tests/fsck-tests/034-odd-inode-flags/default_case.img new file mode 100644 index 0000000000000000000000000000000000000000..43a2a6f62d5ef3afd77f117b577a56ad6098ed19 GIT binary patch literal 4096 zcmeH}c{CJUAIFU?k*yI4S@RGnly%15rj(_~G8rO6vS$b*hR9G^M_GnP*~gG%*XULD zecwiA3?YW>hM5=dd(Ly-=e+&-p7Z|Uo_l`xdw=)dbMEhRes@7VO!Ok2v8iSFcVXRY z0S9$YY#lgx4lMhgmx00fKp$b;YjjKwbl1Mc|4Xm#`|-vHGaC;^56vNgLjwQD1pG!) z0%s{UtRTF}`3%o*f~o}Ub~i7&5a;#~3S~wq>yb%mpwOOBQV@&XDK2I`K}NNkK2T)W zP3ZBKkdz|>(+ppAd?n&e`Mhq5Y_Mx6#)lkr%V0RYfv<lo6x+fvt02ztkuYp%%Q;xT zy4}PY3R{h2>XKS#j!hDNZK|Zl0`#(aZhs{~>g=;i^@`D<CKuSR)XSV?$%1x;Facnk z)_??Qc_w|aw;%(sc8y(>f+(}JrN_k>!De44F)K~Z_B2ji*e|B3TM=T#=3J?hFQkLT zcoHc*_{i*Zl|=_O)<@(9atD)c?k`SQU{$3zUo_)!00YeaVw>N{!sut0FUEY0%ig=a zMor(Rh4`G&3Z`YTQC;Vd8DgP6v&wwJAFZj(Pk4lF8cYH?czMG@QAi}DN<f3p?YI#U zyAG(s@OX8AD?h8M^=`Emq#9RwRZ&U}U7n8To_DtZ^4`G6!3#&+-?VowBGl?;Rjaei zNnff3HtL%3oxdgpDm7yp@ecEi2>#2dOFHE}ny2M`mZ24yt(4GUf&ZQ}VgxH+`7uL@ zKDpXm>?+9{UNN|}x<bse5wSs>P%5x8e<zs@a%|&BuWlvM+i#?{d()aqWBjQEjhabN zYcf1}lwe;gf|^d{H;P_dp);;c-9YV=(1rm`gpVx2(Hs_L@h6Qx`pEbKr7R7?KGxrf zG!wrT)?;kcCsTie%}Dc}a9o*>hlk9$5E{}{L^#S!Q*F3j-dy9VK#9I*B-fuj6aJJl z=P4E0NBM;7Fya(6l|UU2sBHD~_7;trI@t}asks;9Kk+;cNS#DJ)+5qi%9$5}Frqs< zGo#p;!NId;WbB=u&gsfwzoG?K!Wum6^@+(Q%lm<^l5MSYiPzgj6jAV1WhvGO+kDGX zzw5{X`z6v>V}g9P^96Zk*+d9P;fMmiO;S-TF=bQT1)?9j;R){>iIAX>R5$~D?vMpj zBnOV&O)t?w>WW|3jF#YJ{gK;@)e6U&za4XlU`2;z47;3jE!%&}epjB+(;{Nt3NKke zgQOrb4!>S43Sh|5t~6Q9_eALyPQFaH!cTLiXpX=W)Jx7Uxq;~%Hy)VR7GgFu`>T4N zZ?@I};@Zg%oqD*Vk4*qEy@RB5H{Z&f4U<Q;@H#|IMeAITDY?oFt1`Hv_Q+w=rPHS( zs$wMO0~n6+D03e&Eb0?dohu~BjhQu&>>mf%4)?l}C7)7z9RiH3+|z}`sur%ucH}%V zPz+A(a7rcq^2?5ccAa_hN*yFmglI&qBhX31!5N>l2$a={+~R77Z4R7$n%aIEVA=N$ z1b$h$FIy~<-xmLlH6<fH#9zgZtq$4$J^wbe#Iivx5EU*v#Dc<!olL0KOm#aJ^Mx+3 z2p(GjYyfVR`N|kR*X{MSpcQ{~S0_E#n0ldZO1YA(GTpkpg(y?0?s!Uc%eiH_L0*GI za#znPyji<^3MWfMg~bdgcOz~Ooz0a3hOjHtIz&W)(|&`0r__eeAb|X|07Q#}i2ex3 z?C1n<C2VOATuoVj=(1Z*4`hfwss`ALXZ&_#g5!Q#KXigCnU8_lLLnM5ejXsH%XuVC z=KED`QAbQdkC(@Hy%&?P4-3~>n$~>{;68g#pjl7j63c)%xA>Dd9r0^C%QE)RQfgi2 zXiewE(h-{O)QWl=h!TjI+0de3-VC=YSC^0MN(pB5!!V!#68@VzTm&5G-AS>CCye#X zC(0KE`nt?7e5TX|O%{xg!*Ab5P=2^-DRjo81w`|=zUk#|&(uj(4p2-{$h3e`sM17w zdW#Tl75Q}}emQch`~$C9_sjTscj@M>sfi4kHHdcdX*RC$on>k-Dn@x1YE7>^O(DHf z{Sf3DiCn-r)&Jv<Hxip3aQ^=Q80$CQZUB3iMICp?PVBfWGxs3LT(i9*7B(G@)@Eyd zjJ3XwENJZF2P~x;`+Yp=nEbmow1SpkjdDTbjJ6-01QZRo$iJSYsF>e#3Nqt1-yQ4Z zjQbKaB#VjYFLQ=5E+mdcWR7)?7QwYYr_nqX2!rj`wma*sUcEyVH)-UMp9vk5p*sJi z)uP>;M~F@^1@nip{sw)_u!t0m3wfd_^tsEjdl3JV!)ti4tO%j3AZkU&1P8pdqT2@4 z)2a@Bm48L_Y|X>TZU*bQ)wNAuogJWSw*}wLwwe>k*K*tA6k?IBPtx`FWcq8tfigF_ z<a|;3O1QIssZv+n#5SG!^V4Y&|D<(IbjP@$g*$9F*lA~*K1>`^-m<5?dEeNYIhR9? zpXv|E5a?(uK@>aY^((xBosb-{GjPf)k&M1J(;|7FUGk~Pl7(`D#yu_syY49#0sXEQ zq2Yh_;mDS0F#bl9X5`NV5q91498spY6Yu}FrF1zva3DCCYrYxh$_DQ7>Kut=Pt1kZ z=<FI^nlnaeh6Ounf6x!Agfw_3ztH51r`(~fWPHIJE#sScog}q#^ZcNjkkDWT0}Nqt zFXuZ-H~@Ah8<%Hn^c0iZ%hSJRE1;Ex$fh(N+4l)MfFwEOWtF%@T~=vDOV<PkekL!A ztK!oXCx6Vu3Wzdl{TODlh3_j(oCW~}8?a*%m2<5!1!L5f2Zaf9CGC!AH#pEYvq#nn zFmu02{qC|`H7=bqHYo?_i^}rkmN0v4k5<iF_OGA_&X0I~s^tv*%{7XL)6hSG5Q~X( kIq6x%Jnm_FaB;gC4ZY2N*-{?-H$sQqLjs2c{x<~v38kbqN&o-= literal 0 HcmV?d00001 diff --git a/tests/fsck-tests/034-odd-inode-flags/test.sh b/tests/fsck-tests/034-odd-inode-flags/test.sh new file mode 100755 index 000000000000..2225efb44e0e --- /dev/null +++ b/tests/fsck-tests/034-odd-inode-flags/test.sh @@ -0,0 +1,15 @@ +#!/bin/bash +# In order to confirm that 'btrfs check' supports checking symlinks +# with immutable/append attributes. +# + +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.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-05-14 12:12 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-14 10:29 [PATCH 0/3] btrfs-progs: check: verify symlinks with append/immutable flags Su Yue 2018-05-14 10:29 ` [PATCH 1/3] btrfs-progs: check: check " Su Yue 2018-05-14 11:18 ` Nikolay Borisov 2018-05-14 12:12 ` Su Yue 2018-05-14 11:22 ` Qu Wenruo 2018-05-14 11:52 ` Su Yue 2018-05-14 10:29 ` [PATCH 2/3] btrfs-progs: lowmem: " Su Yue 2018-05-14 11:17 ` Nikolay Borisov 2018-05-14 12:04 ` Su Yue 2018-05-14 11:27 ` Qu Wenruo 2018-05-14 10:29 ` [PATCH 3/3] btrfs-progs: fsck-tests: add test case to check symlinks with odd flags Su Yue
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.