Re: [bug report] fsdax: output address in dax_iomap_pfn() and rename it

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





在 2022/6/7 15:59, Dan Carpenter 写道:
Hello Shiyang Ruan,

The patch 1447ac26a964: "fsdax: output address in dax_iomap_pfn() and
rename it" from Jun 3, 2022, leads to the following Smatch static
checker warning:

	fs/dax.c:1085 dax_iomap_direct_access()
	error: uninitialized symbol 'rc'.

fs/dax.c
     1052 static int dax_iomap_direct_access(const struct iomap *iomap, loff_t pos,
     1053                 size_t size, void **kaddr, pfn_t *pfnp)
     1054 {
     1055         pgoff_t pgoff = dax_iomap_pgoff(iomap, pos);
     1056         int id, rc;
     1057         long length;
     1058
     1059         id = dax_read_lock();
     1060         length = dax_direct_access(iomap->dax_dev, pgoff, PHYS_PFN(size),
     1061                                    DAX_ACCESS, kaddr, pfnp);
     1062         if (length < 0) {
     1063                 rc = length;
     1064                 goto out;
     1065         }
     1066         if (!pfnp)
     1067                 goto out_check_addr;

Is this an error path?

     1068         rc = -EINVAL;
     1069         if (PFN_PHYS(length) < size)
     1070                 goto out;
     1071         if (pfn_t_to_pfn(*pfnp) & (PHYS_PFN(size)-1))
     1072                 goto out;
     1073         /* For larger pages we need devmap */
     1074         if (length > 1 && !pfn_t_devmap(*pfnp))
     1075                 goto out;
     1076         rc = 0;
     1077
     1078 out_check_addr:
     1079         if (!kaddr)
     1080                 goto out;

How is it supposed to be handled if both "pfnp" and "kaddr" are NULL?

Smatch says that "kaddr" can never be NULL so this code is just future
proofing but I didn't look at it carefully.

Yes, we always pass the @kaddr in all caller, it won't be NULL now. And even @kaddr and @pfnp are both NULL, it won't cause any error. So, I think the rc should be initialized to 0 :

 {
        pgoff_t pgoff = dax_iomap_pgoff(iomap, pos);
-       int id, rc;
+       int id, rc = 0;
        long length;

Do I need to fix this and resend a patch to the list? Or you could help me fix this?


--
Thanks,
Ruan.


     1081         if (!*kaddr)
     1082                 rc = -EFAULT;
     1083 out:
     1084         dax_read_unlock(id);
--> 1085         return rc;
     1086 }

regards,
dan carpenter





[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux