* [bug report] fs/ntfs3: Add file operations and implementation
@ 2021-08-24 9:02 Dan Carpenter
2021-08-24 9:32 ` Kari Argillander
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2021-08-24 9:02 UTC (permalink / raw)
To: almaz.alexandrovich; +Cc: ntfs3
Hello Konstantin Komarov,
This is a semi-automatic email about new static checker warnings.
The patch 4342306f0f0d: "fs/ntfs3: Add file operations and
implementation" from Aug 13, 2021, leads to the following Smatch
complaint:
fs/ntfs3/namei.c:446 ntfs_rename()
warn: variable dereferenced before check 'old_inode' (see line 312)
fs/ntfs3/namei.c
311
312 if (ntfs_is_meta_file(sbi, old_inode->i_ino)) {
^^^^^^^^^^^^^^^^
Dereference
313 err = -EINVAL;
314 goto out;
315 }
316
317 if (new_inode) {
318 /*target name exists. unlink it*/
319 dget(new_dentry);
320 ni_lock_dir(new_dir_ni);
321 err = ntfs_unlink_inode(new_dir, new_dentry);
322 ni_unlock(new_dir_ni);
323 dput(new_dentry);
324 if (err)
325 goto out;
326 }
327
328 /* allocate PATH_MAX bytes */
329 old_de = __getname();
330 if (!old_de) {
331 err = -ENOMEM;
332 goto out;
333 }
334
335 err = fill_name_de(sbi, old_de, &old_dentry->d_name, NULL);
336 if (err < 0)
337 goto out1;
338
339 old_name = (struct ATTR_FILE_NAME *)(old_de + 1);
340
341 if (is_same) {
342 new_de = old_de;
343 } else {
344 new_de = Add2Ptr(old_de, 1024);
345 err = fill_name_de(sbi, new_de, &new_dentry->d_name, NULL);
346 if (err < 0)
347 goto out1;
348 }
349
350 ni_lock_dir(old_dir_ni);
351 ni_lock(old_ni);
352
353 mi_get_ref(&old_dir_ni->mi, &old_name->home);
354
355 /*get pointer to file_name in mft*/
356 fname = ni_fname_name(old_ni, (struct cpu_str *)&old_name->name_len,
357 &old_name->home, &le);
358 if (!fname) {
359 err = -EINVAL;
360 goto out2;
361 }
362
363 /* Copy fname info from record into new fname */
364 new_name = (struct ATTR_FILE_NAME *)(new_de + 1);
365 memcpy(&new_name->dup, &fname->dup, sizeof(fname->dup));
366
367 name_type = paired_name(fname->type);
368
369 /* remove first name from directory */
370 err = indx_delete_entry(&old_dir_ni->dir, old_dir_ni, old_de + 1,
371 le16_to_cpu(old_de->key_size), sbi);
372 if (err)
373 goto out3;
374
375 /* remove first name from mft */
376 err = ni_remove_attr_le(old_ni, attr_from_name(fname), le);
377 if (err)
378 goto out4;
379
380 le16_add_cpu(&old_ni->mi.mrec->hard_links, -1);
381 old_ni->mi.dirty = true;
382
383 if (name_type != FILE_NAME_POSIX) {
384 /* get paired name */
385 fname = ni_fname_type(old_ni, name_type, &le);
386 if (fname) {
387 /* remove second name from directory */
388 err = indx_delete_entry(&old_dir_ni->dir, old_dir_ni,
389 fname, fname_full_size(fname),
390 sbi);
391 if (err)
392 goto out5;
393
394 /* remove second name from mft */
395 err = ni_remove_attr_le(old_ni, attr_from_name(fname),
396 le);
397 if (err)
398 goto out6;
399
400 le16_add_cpu(&old_ni->mi.mrec->hard_links, -1);
401 old_ni->mi.dirty = true;
402 }
403 }
404
405 /* Add new name */
406 mi_get_ref(&old_ni->mi, &new_de->ref);
407 mi_get_ref(&ntfs_i(new_dir)->mi, &new_name->home);
408
409 new_de_key_size = le16_to_cpu(new_de->key_size);
410
411 /* insert new name in mft */
412 err = ni_insert_resident(old_ni, new_de_key_size, ATTR_NAME, NULL, 0,
413 &attr, NULL);
414 if (err)
415 goto out7;
416
417 attr->res.flags = RESIDENT_FLAG_INDEXED;
418
419 memcpy(Add2Ptr(attr, SIZEOF_RESIDENT), new_name, new_de_key_size);
420
421 le16_add_cpu(&old_ni->mi.mrec->hard_links, 1);
422 old_ni->mi.dirty = true;
423
424 /* insert new name in directory */
425 err = indx_insert_entry(&new_dir_ni->dir, new_dir_ni, new_de, sbi,
426 NULL);
427 if (err)
428 goto out8;
429
430 if (IS_DIRSYNC(new_dir))
431 err = ntfs_sync_inode(old_inode);
432 else
433 mark_inode_dirty(old_inode);
434
435 old_dir->i_ctime = old_dir->i_mtime = current_time(old_dir);
436 if (IS_DIRSYNC(old_dir))
437 (void)ntfs_sync_inode(old_dir);
438 else
439 mark_inode_dirty(old_dir);
440
441 if (old_dir != new_dir) {
442 new_dir->i_mtime = new_dir->i_ctime = old_dir->i_ctime;
443 mark_inode_dirty(new_dir);
444 }
445
446 if (old_inode) {
^^^^^^^^^
If old_inode can be NULL we are toasted.
447 old_inode->i_ctime = old_dir->i_ctime;
448 mark_inode_dirty(old_inode);
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] fs/ntfs3: Add file operations and implementation
2021-08-24 9:02 [bug report] fs/ntfs3: Add file operations and implementation Dan Carpenter
@ 2021-08-24 9:32 ` Kari Argillander
0 siblings, 0 replies; 2+ messages in thread
From: Kari Argillander @ 2021-08-24 9:32 UTC (permalink / raw)
To: Dan Carpenter; +Cc: almaz.alexandrovich, ntfs3
On Tue, Aug 24, 2021 at 12:02:55PM +0300, Dan Carpenter wrote:
> Hello Konstantin Komarov,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch 4342306f0f0d: "fs/ntfs3: Add file operations and
> implementation" from Aug 13, 2021, leads to the following Smatch
> complaint:
>
> fs/ntfs3/namei.c:446 ntfs_rename()
> warn: variable dereferenced before check 'old_inode' (see line 312)
>
> fs/ntfs3/namei.c
> 311
> 312 if (ntfs_is_meta_file(sbi, old_inode->i_ino)) {
> ^^^^^^^^^^^^^^^^
> Dereference
>
> 313 err = -EINVAL;
> 314 goto out;
> 315 }
> 316
> 317 if (new_inode) {
> 318 /*target name exists. unlink it*/
> 319 dget(new_dentry);
> 320 ni_lock_dir(new_dir_ni);
> 321 err = ntfs_unlink_inode(new_dir, new_dentry);
> 322 ni_unlock(new_dir_ni);
> 323 dput(new_dentry);
> 324 if (err)
> 325 goto out;
> 326 }
> 327
> 328 /* allocate PATH_MAX bytes */
> 329 old_de = __getname();
> 330 if (!old_de) {
> 331 err = -ENOMEM;
> 332 goto out;
> 333 }
> 334
> 335 err = fill_name_de(sbi, old_de, &old_dentry->d_name, NULL);
> 336 if (err < 0)
> 337 goto out1;
> 338
> 339 old_name = (struct ATTR_FILE_NAME *)(old_de + 1);
> 340
> 341 if (is_same) {
> 342 new_de = old_de;
> 343 } else {
> 344 new_de = Add2Ptr(old_de, 1024);
> 345 err = fill_name_de(sbi, new_de, &new_dentry->d_name, NULL);
> 346 if (err < 0)
> 347 goto out1;
> 348 }
> 349
> 350 ni_lock_dir(old_dir_ni);
> 351 ni_lock(old_ni);
> 352
> 353 mi_get_ref(&old_dir_ni->mi, &old_name->home);
> 354
> 355 /*get pointer to file_name in mft*/
> 356 fname = ni_fname_name(old_ni, (struct cpu_str *)&old_name->name_len,
> 357 &old_name->home, &le);
> 358 if (!fname) {
> 359 err = -EINVAL;
> 360 goto out2;
> 361 }
> 362
> 363 /* Copy fname info from record into new fname */
> 364 new_name = (struct ATTR_FILE_NAME *)(new_de + 1);
> 365 memcpy(&new_name->dup, &fname->dup, sizeof(fname->dup));
> 366
> 367 name_type = paired_name(fname->type);
> 368
> 369 /* remove first name from directory */
> 370 err = indx_delete_entry(&old_dir_ni->dir, old_dir_ni, old_de + 1,
> 371 le16_to_cpu(old_de->key_size), sbi);
> 372 if (err)
> 373 goto out3;
> 374
> 375 /* remove first name from mft */
> 376 err = ni_remove_attr_le(old_ni, attr_from_name(fname), le);
> 377 if (err)
> 378 goto out4;
> 379
> 380 le16_add_cpu(&old_ni->mi.mrec->hard_links, -1);
> 381 old_ni->mi.dirty = true;
> 382
> 383 if (name_type != FILE_NAME_POSIX) {
> 384 /* get paired name */
> 385 fname = ni_fname_type(old_ni, name_type, &le);
> 386 if (fname) {
> 387 /* remove second name from directory */
> 388 err = indx_delete_entry(&old_dir_ni->dir, old_dir_ni,
> 389 fname, fname_full_size(fname),
> 390 sbi);
> 391 if (err)
> 392 goto out5;
> 393
> 394 /* remove second name from mft */
> 395 err = ni_remove_attr_le(old_ni, attr_from_name(fname),
> 396 le);
> 397 if (err)
> 398 goto out6;
> 399
> 400 le16_add_cpu(&old_ni->mi.mrec->hard_links, -1);
> 401 old_ni->mi.dirty = true;
> 402 }
> 403 }
> 404
> 405 /* Add new name */
> 406 mi_get_ref(&old_ni->mi, &new_de->ref);
> 407 mi_get_ref(&ntfs_i(new_dir)->mi, &new_name->home);
> 408
> 409 new_de_key_size = le16_to_cpu(new_de->key_size);
> 410
> 411 /* insert new name in mft */
> 412 err = ni_insert_resident(old_ni, new_de_key_size, ATTR_NAME, NULL, 0,
> 413 &attr, NULL);
> 414 if (err)
> 415 goto out7;
> 416
> 417 attr->res.flags = RESIDENT_FLAG_INDEXED;
> 418
> 419 memcpy(Add2Ptr(attr, SIZEOF_RESIDENT), new_name, new_de_key_size);
> 420
> 421 le16_add_cpu(&old_ni->mi.mrec->hard_links, 1);
> 422 old_ni->mi.dirty = true;
> 423
> 424 /* insert new name in directory */
> 425 err = indx_insert_entry(&new_dir_ni->dir, new_dir_ni, new_de, sbi,
> 426 NULL);
> 427 if (err)
> 428 goto out8;
> 429
> 430 if (IS_DIRSYNC(new_dir))
> 431 err = ntfs_sync_inode(old_inode);
> 432 else
> 433 mark_inode_dirty(old_inode);
> 434
> 435 old_dir->i_ctime = old_dir->i_mtime = current_time(old_dir);
> 436 if (IS_DIRSYNC(old_dir))
> 437 (void)ntfs_sync_inode(old_dir);
> 438 else
> 439 mark_inode_dirty(old_dir);
> 440
> 441 if (old_dir != new_dir) {
> 442 new_dir->i_mtime = new_dir->i_ctime = old_dir->i_ctime;
> 443 mark_inode_dirty(new_dir);
> 444 }
> 445
> 446 if (old_inode) {
> ^^^^^^^^^
> If old_inode can be NULL we are toasted.
It seems to me that old_inode cannot be NULL. So this check is not
needed.
>
> 447 old_inode->i_ctime = old_dir->i_ctime;
> 448 mark_inode_dirty(old_inode);
>
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-08-24 9:32 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-24 9:02 [bug report] fs/ntfs3: Add file operations and implementation Dan Carpenter
2021-08-24 9:32 ` Kari Argillander
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).