Re: [bug report] fs/ntfs3: Add attrib operations

Kari Argillander
2021-08-24T10:49:14Z

On Tue, Aug 24, 2021 at 12:42:49PM +0300, Dan Carpenter wrote:
> Hello Konstantin Komarov,
>
> The patch be71b5cba2e6: "fs/ntfs3: Add attrib operations" from Aug
> 13, 2021, leads to the following
> Smatch static checker warning:
>
> fs/ntfs3/attrib.c:383 attr_set_size_res()
> warn: was expecting a 64 bit value instead of '~7'
>
> fs/ntfs3/attrib.c
> 370 static int attr_set_size_res(struct ntfs_inode *ni, struct ATTRIB *attr,
> 371 struct ATTR_LIST_ENTRY *le, struct mft_inode *mi,
> 372 u64 new_size, struct runs_tree *run,
> 373 struct ATTRIB **ins_attr)
> 374 {
> 375 struct ntfs_sb_info *sbi = mi->sbi;
> 376 struct MFT_REC *rec = mi->mrec;
> 377 u32 used = le32_to_cpu(rec->used);
> 378 u32 asize = le32_to_cpu(attr->size);
> 379 u32 aoff = PtrOffset(rec, attr);
> 380 u32 rsize = le32_to_cpu(attr->res.data_size);
> 381 u32 tail = used - aoff - asize;
> 382 char *next = Add2Ptr(attr, asize);
> --> 383 s64 dsize = QuadAlign(new_size) - QuadAlign(rsize);
> ^^^^^^^^^^^^^^^^^^
> QuadAlign() is a bad name.

Agreed. I will make patch to this someday if nobody else haven't done it
already by then. These kind of macros can confuse checkpatch and
coccinelle so that is one good reason to get away from them.

>
> The ntfs3 code has a bunch of bad macros like ntfs_malloc() which will
> need to be removed hopefully? I haven't seen this code before today but
> presumably everyone has mentioned this already.

ntfs_malloc has already been addressed and patch is made.

lore.kernel.org/ntfs3/20210818010649.412912-1-kari.argillander@gmail.com/

>
> Anyway, new_size is a u64 and QuadAlign() truncates it to u32. Use the
> normal ALIGN() macro.
>
> 384
> 385 if (dsize < 0) {
> 386 memmove(next + dsize, next, tail);
> 387 } else if (dsize > 0) {
> 388 if (used + dsize > sbi->max_bytes_per_attr)
> 389 return attr_make_nonresident(ni, attr, le, mi, new_size,
> 390 run, ins_attr, NULL);
> 391
> 392 memmove(next + dsize, next, tail);
> 393 memset(next, 0, dsize);
> 394 }
> 395
> 396 if (new_size > rsize)
> 397 memset(Add2Ptr(resident_data(attr), rsize), 0,
> 398 new_size - rsize);
> 399
> 400 rec->used = cpu_to_le32(used + dsize);
> 401 attr->size = cpu_to_le32(asize + dsize);
> 402 attr->res.data_size = cpu_to_le32(new_size);
> 403 mi->dirty = true;
> 404 *ins_attr = attr;
> 405
> 406 return 0;
> 407 }
>
> regards,
> dan carpenter
>