Hi Konstantin, On Wed, Apr 17, 2024 at 04:09:00PM +0300, Konstantin Komarov wrote: > Add more checks using $AttrDef. > > Signed-off-by: Konstantin Komarov <almaz.alexandrovich@xxxxxxxxxxxxxxxxxxxx> <snip> > diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c > index 8beefbca5769..dae961d2d6f8 100644 > --- a/fs/ntfs3/super.c > +++ b/fs/ntfs3/super.c > @@ -481,11 +481,39 @@ static int ntfs3_volinfo_open(struct inode *inode, > struct file *file) > /* read /proc/fs/ntfs3/<dev>/label */ > static int ntfs3_label_show(struct seq_file *m, void *o) > { > + int len; > struct super_block *sb = m->private; > struct ntfs_sb_info *sbi = sb->s_fs_info; > + struct ATTRIB *attr; > + u8 *label = kmalloc(PAGE_SIZE, GFP_NOFS); > + > + if (!label) > + return -ENOMEM; > + > + attr = ni_find_attr(sbi->volume.ni, NULL, NULL, ATTR_LABEL, NULL, 0, > + NULL, NULL); > + > + if (!attr) { > + /* It is ok if no ATTR_LABEL */ > + label[0] = 0; > + len = 0; > + } else if (!attr_check(attr, sbi, sbi->volume.ni)) { > + len = sprintf(label, "%pg: failed to get label", sb->s_bdev); > + } else { > + len = ntfs_utf16_to_nls(sbi, resident_data(attr), > + le32_to_cpu(attr->res.data_size) >> 1, > + label, PAGE_SIZE); > + if (len < 0) { > + label[0] = 0; > + len = 0; > + } else if (len >= PAGE_SIZE) { > + len = PAGE_SIZE - 1; > + } > + } > > - seq_printf(m, "%s\n", sbi->volume.label); > + seq_printf(m, "%.*s\n", len, label); > > + kfree(label); > return 0; > } > > @@ -1210,25 +1238,6 @@ static int ntfs_fill_super(struct super_block *sb, > struct fs_context *fc) > > ni = ntfs_i(inode); > > - /* Load and save label (not necessary). */ > - attr = ni_find_attr(ni, NULL, NULL, ATTR_LABEL, NULL, 0, NULL, NULL); > - > - if (!attr) { > - /* It is ok if no ATTR_LABEL */ > - } else if (!attr->non_res && !is_attr_ext(attr)) { > - /* $AttrDef allows labels to be up to 128 symbols. */ > - err = utf16s_to_utf8s(resident_data(attr), > - le32_to_cpu(attr->res.data_size) >> 1, > - UTF16_LITTLE_ENDIAN, sbi->volume.label, > - sizeof(sbi->volume.label)); > - if (err < 0) > - sbi->volume.label[0] = 0; > - } else { > - /* Should we break mounting here? */ > - //err = -EINVAL; > - //goto put_inode_out; > - } > - The attr initialization above causes the use in the ni_find_attr() to be uninitialized, which results in the following clang warning (or error when CONFIG_WERROR is enabled): fs/ntfs3/super.c:1247:26: error: variable 'attr' is uninitialized when used here [-Werror,-Wuninitialized] 1247 | attr = ni_find_attr(ni, attr, NULL, ATTR_VOL_INFO, NULL, 0, NULL, NULL); | ^~~~ fs/ntfs3/super.c:1192:21: note: initialize the variable 'attr' to silence this warning 1192 | struct ATTRIB *attr; | ^ | = NULL 1 error generated. Please either fix this quickly (as this isn't the first time an ntfs3 change has broken us for some time in -next for a similar reason [1]) or remove this series from -next. Given the issues that Johan has brought up in other comments in this thread, I feel like the latter may be better, as this series is clearly not ready for Linus (which is one of -next's general requirements AFAIUI). > attr = ni_find_attr(ni, attr, NULL, ATTR_VOL_INFO, NULL, 0, NULL, > NULL); > if (!attr || is_attr_ext(attr) || > !(info = resident_data_ex(attr, SIZEOF_ATTRIBUTE_VOLUME_INFO))) { [1]: https://github.com/ClangBuiltLinux/linux/issues/1729 Cheers, Nathan