On 6.09.19 г. 12:58 ч., Vladimir Panteleev wrote:
> Use the return value of listxattr instead of tokenizing.
>
> The end of the extended attribute list is indicated by the return
> value, not an empty list item (two consecutive NULs). Using strtok
> in this way thus sometimes caused add_xattr_item to reuse stack data
> in xattr_list from the previous invocation, thus querying attributes
> that are not actually in the file's xattr list.
>
> Issue: #194
> Signed-off-by: Vladimir Panteleev <git@xxxxxxxxxxxxxxxxxxxxx>
Can you elaborate how to trigger this? I tried by creating a folder with
2 files and set 5 xattr to the first file and 1 to the second. Then I
run mkfs.btrfs -r /path/to/dir /dev/vdc and stepped through the code
with gdb and didn't see any issues. Ideally I'd like to see a regression
test for this issue.
Your code looks correct.
> ---
> mkfs/rootdir.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/mkfs/rootdir.c b/mkfs/rootdir.c
> index 51411e02..c86159e7 100644
> --- a/mkfs/rootdir.c
> +++ b/mkfs/rootdir.c
> @@ -228,10 +228,9 @@ static int add_xattr_item(struct btrfs_trans_handle *trans,
> int ret;
> int cur_name_len;
> char xattr_list[XATTR_LIST_MAX];
> + char *xattr_list_end;
> char *cur_name;
> char cur_value[XATTR_SIZE_MAX];
> - char delimiter = '\0';
> - char *next_location = xattr_list;
>
> ret = llistxattr(file_name, xattr_list, XATTR_LIST_MAX);
> if (ret < 0) {
> @@ -243,10 +242,10 @@ static int add_xattr_item(struct btrfs_trans_handle *trans,
> if (ret == 0)
> return ret;
>
> - cur_name = strtok(xattr_list, &delimiter);
> - while (cur_name != NULL) {
> + xattr_list_end = xattr_list + ret;
> + cur_name = xattr_list;
> + while (cur_name < xattr_list_end) {
> cur_name_len = strlen(cur_name);
> - next_location += cur_name_len + 1;
>
> ret = lgetxattr(file_name, cur_name, cur_value, XATTR_SIZE_MAX);
> if (ret < 0) {
> @@ -266,7 +265,7 @@ static int add_xattr_item(struct btrfs_trans_handle *trans,
> file_name);
> }
>
> - cur_name = strtok(next_location, &delimiter);
> + cur_name += cur_name_len + 1;
> }
>
> return ret;
>
