| Message ID | 20170601085716.25898-7-suy.fnst@cn.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 1.06.2017 11:57, Su Yue wrote: > Since 'iterate_dir_item' checks namelen in its way, > use 'btrfs_is_namelen_valid' not 'verify_dir_item'. > > Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> > --- > fs/btrfs/send.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c > index fc496a6f842a..caf96af106e6 100644 > --- a/fs/btrfs/send.c > +++ b/fs/btrfs/send.c > @@ -1069,6 +1069,12 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path, > } > } > > + ret = btrfs_is_namelen_valid(eb, path->slots[0], > + (unsigned long)(di + 1), name_len + data_len); > + if (!ret) { > + ret = -ENAMETOOLONG; In 5/9 and 7/9 the return values upon btrfs_is_namelen_valid failure are different. Shouldn't the failure root cause (corrupted datastructures) always be the same when btrfs_is_namelen_valid fails? E.g. in the case of send we shouldn't really have entries which are ENAMETOOLONG, since they should've been rejected at time they were originally created with ENAMETOOLONG. And in case corruption happened and iterate_dir_item observes failure from btrfs_is_namelen_valid then this should be EIO/EUCLEAN ? > + goto out; > + } > if (name_len + data_len > buf_len) { > buf_len = name_len + data_len; > if (is_vmalloc_addr(buf)) { > -- 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
On Thu, Jun 01, 2017 at 12:58:12PM +0300, Nikolay Borisov wrote: > > > On 1.06.2017 11:57, Su Yue wrote: > > Since 'iterate_dir_item' checks namelen in its way, > > use 'btrfs_is_namelen_valid' not 'verify_dir_item'. > > > > Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> > > --- > > fs/btrfs/send.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c > > index fc496a6f842a..caf96af106e6 100644 > > --- a/fs/btrfs/send.c > > +++ b/fs/btrfs/send.c > > @@ -1069,6 +1069,12 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path, > > } > > } > > > > + ret = btrfs_is_namelen_valid(eb, path->slots[0], > > + (unsigned long)(di + 1), name_len + data_len); > > + if (!ret) { > > + ret = -ENAMETOOLONG; > > In 5/9 and 7/9 the return values upon btrfs_is_namelen_valid failure are > different. Shouldn't the failure root cause (corrupted datastructures) > always be the same when btrfs_is_namelen_valid fails? E.g. in the case > of send we shouldn't really have entries which are ENAMETOOLONG, since > they should've been rejected at time they were originally created with > ENAMETOOLONG. And in case corruption happened and iterate_dir_item > observes failure from btrfs_is_namelen_valid then this should be > EIO/EUCLEAN ? Agreed. -- 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
diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index fc496a6f842a..caf96af106e6 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -1069,6 +1069,12 @@ static int iterate_dir_item(struct btrfs_root *root, struct btrfs_path *path, } } + ret = btrfs_is_namelen_valid(eb, path->slots[0], + (unsigned long)(di + 1), name_len + data_len); + if (!ret) { + ret = -ENAMETOOLONG; + goto out; + } if (name_len + data_len > buf_len) { buf_len = name_len + data_len; if (is_vmalloc_addr(buf)) {
Since 'iterate_dir_item' checks namelen in its way, use 'btrfs_is_namelen_valid' not 'verify_dir_item'. Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com> --- fs/btrfs/send.c | 6 ++++++ 1 file changed, 6 insertions(+)