[bug report] fs/ntfs3: Add attrib operations

Dan Carpenter
2021-08-24T09:53:24Z

13, 2021, leads to the following
Smatch static checker warning:

fs/ntfs3/attrib.c:882 attr_data_get_block()
warn: was expecting a 64 bit value instead of '~(clst_per_frame - 1)'

fs/ntfs3/attrib.c
823 int attr_data_get_block(struct ntfs_inode *ni, CLST vcn, CLST clen, CLST *lcn,
824 CLST *len, bool *new)
825 {
826 int err = 0;
827 struct runs_tree *run = &ni->file.run;
828 struct ntfs_sb_info *sbi;
829 u8 cluster_bits;
830 struct ATTRIB *attr = NULL, *attr_b;
831 struct ATTR_LIST_ENTRY *le, *le_b;
832 struct mft_inode *mi, *mi_b;
833 CLST hint, svcn, to_alloc, evcn1, next_svcn, asize, end;
834 u64 total_size;
835 u32 clst_per_frame;
836 bool ok;
837
838 if (new)
839 *new = false;
840
841 down_read(&ni->file.run_lock);
842 ok = run_lookup_entry(run, vcn, lcn, len, NULL);
843 up_read(&ni->file.run_lock);
844
845 if (ok && (*lcn != SPARSE_LCN || !new)) {
846 /* normal way */
847 return 0;
848 }
849
850 if (!clen)
851 clen = 1;
852
853 if (ok && clen > *len)
854 clen = *len;
855
856 sbi = ni->mi.sbi;
857 cluster_bits = sbi->cluster_bits;
858
859 ni_lock(ni);
860 down_write(&ni->file.run_lock);
861
862 le_b = NULL;
863 attr_b = ni_find_attr(ni, NULL, &le_b, ATTR_DATA, NULL, 0, NULL, &mi_b);
864 if (!attr_b) {
865 err = -ENOENT;
866 goto out;
867 }
868
869 if (!attr_b->non_res) {
870 *lcn = RESIDENT_LCN;
871 *len = 1;
872 goto out;
873 }
874
875 asize = le64_to_cpu(attr_b->nres.alloc_size) >> sbi->cluster_bits;
876 if (vcn >= asize) {
877 err = -EINVAL;
878 goto out;
879 }
880
881 clst_per_frame = 1u << attr_b->nres.c_unit;
--> 882 to_alloc = (clen + clst_per_frame - 1) & ~(clst_per_frame - 1);

In this config "clen" is a u64 and "clst_per_frame" is a u32 so this
code will truncate to_alloc to a u32. An easy fix is to use the
ALIGN() macro.

to_alloc = ALIGN(clen, clst_per_frame);

However, I still had some questions below so I did not write a patch.

883
884 if (vcn + to_alloc > asize)
885 to_alloc = asize - vcn;

If to_alloc is too large for asize then make it smaller. Is it still
ALIGNED?

886
887 svcn = le64_to_cpu(attr_b->nres.svcn);
888 evcn1 = le64_to_cpu(attr_b->nres.evcn) + 1;
889
890 attr = attr_b;
891 le = le_b;
892 mi = mi_b;
893
894 if (le_b && (vcn < svcn || evcn1 <= vcn)) {
895 attr = ni_find_attr(ni, attr_b, &le, ATTR_DATA, NULL, 0, &vcn,
896 &mi);
897 if (!attr) {
898 err = -EINVAL;
899 goto out;
900 }
901 svcn = le64_to_cpu(attr->nres.svcn);
902 evcn1 = le64_to_cpu(attr->nres.evcn) + 1;
903 }
904
905 err = attr_load_runs(attr, ni, run, NULL);
906 if (err)
907 goto out;
908
909 if (!ok) {
910 ok = run_lookup_entry(run, vcn, lcn, len, NULL);
911 if (ok && (*lcn != SPARSE_LCN || !new)) {
912 /* normal way */
913 err = 0;
914 goto ok;
915 }
916
917 if (!ok && !new) {
918 *len = 0;
919 err = 0;
920 goto ok;
921 }
922
923 if (ok && clen > *len) {
924 clen = *len;
925 to_alloc = (clen + clst_per_frame - 1) &
926 ~(clst_per_frame - 1);

We re-assign to_alloc here. And it's smaller than before, but it hasn't
been checked against asize so it might not be small enough?

927 }
928 }
929
930 if (!is_attr_ext(attr_b)) {
931 err = -EINVAL;
932 goto out;
933 }
934