[bug report] nfsd: Implement large extent array support in pNFS

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

 



Hello Sergey Bashirov,

Commit 067b594beb16 ("nfsd: Implement large extent array support in
pNFS") from Jun 21, 2025 (linux-next), leads to the following Smatch
static checker warning:

	fs/nfsd/blocklayoutxdr.c:160 nfsd4_block_decode_layoutupdate()
	warn: error code type promoted to positive: 'ret'

fs/nfsd/blocklayoutxdr.c
    135 nfsd4_block_decode_layoutupdate(struct xdr_stream *xdr, struct iomap **iomapp,
    136                 int *nr_iomapsp, u32 block_size)
    137 {
    138         struct iomap *iomaps;
    139         u32 nr_iomaps, expected, len, i;
    140         __be32 nfserr;
    141 
    142         if (xdr_stream_decode_u32(xdr, &nr_iomaps))
    143                 return nfserr_bad_xdr;
    144 
    145         len = sizeof(__be32) + xdr_stream_remaining(xdr);
    146         expected = sizeof(__be32) + nr_iomaps * PNFS_BLOCK_EXTENT_SIZE;
    147         if (len != expected)
    148                 return nfserr_bad_xdr;
    149 
    150         iomaps = kcalloc(nr_iomaps, sizeof(*iomaps), GFP_KERNEL);
    151         if (!iomaps)
    152                 return nfserr_delay;
    153 
    154         for (i = 0; i < nr_iomaps; i++) {
    155                 struct pnfs_block_extent bex;
    156                 ssize_t ret;
    157 
    158                 ret = xdr_stream_decode_opaque_fixed(xdr,
    159                                 &bex.vol_id, sizeof(bex.vol_id));
--> 160                 if (ret < sizeof(bex.vol_id)) {

xdr_stream_decode_opaque_fixed() returns negative error codes or
sizeof(bex.vol_id) on success.  If it returns a negative then the
condition is type promoted to size_t and the negative becomes a high
positive value and is treated as success.

There are so many ways to fix this bug.

#1: if (ret < 0 || ret < sizeof(bex.vol_id)) {
I love this approach but every other person in the world seems to hate
it.

#2: if (ret < (int)sizeof(bex.vol_id)) {
Fine.  I don't love casts, but fine.

#3: if (ret != sizeof(bex.vol_id)) {
I like this well enough.

#4: Change xdr_stream_decode_opaque_fixed() to return zero on success.
This is the best fix.

regards,
dan carpenter

    161                         nfserr = nfserr_bad_xdr;
    162                         goto fail;
    163                 }
    164 
    165                 if (xdr_stream_decode_u64(xdr, &bex.foff)) {
    166                         nfserr = nfserr_bad_xdr;
    167                         goto fail;
    168                 }
    169                 if (bex.foff & (block_size - 1)) {
    170                         nfserr = nfserr_inval;
    171                         goto fail;
    172                 }
    173 
    174                 if (xdr_stream_decode_u64(xdr, &bex.len)) {
    175                         nfserr = nfserr_bad_xdr;
    176                         goto fail;
    177                 }
    178                 if (bex.len & (block_size - 1)) {
    179                         nfserr = nfserr_inval;
    180                         goto fail;
    181                 }
    182 
    183                 if (xdr_stream_decode_u64(xdr, &bex.soff)) {
    184                         nfserr = nfserr_bad_xdr;
    185                         goto fail;
    186                 }
    187                 if (bex.soff & (block_size - 1)) {
    188                         nfserr = nfserr_inval;
    189                         goto fail;
    190                 }
    191 
    192                 if (xdr_stream_decode_u32(xdr, &bex.es)) {
    193                         nfserr = nfserr_bad_xdr;
    194                         goto fail;
    195                 }
    196                 if (bex.es != PNFS_BLOCK_READWRITE_DATA) {
    197                         nfserr = nfserr_inval;
    198                         goto fail;
    199                 }
    200 
    201                 iomaps[i].offset = bex.foff;
    202                 iomaps[i].length = bex.len;
    203         }
    204 
    205         *iomapp = iomaps;
    206         *nr_iomapsp = nr_iomaps;
    207         return nfs_ok;
    208 fail:
    209         kfree(iomaps);
    210         return nfserr;
    211 }





[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux