From: Dan Carpenter <dan.carpenter@linaro.org>
To: Sergey Bashirov <sergeybashirov@gmail.com>
Cc: linux-nfs@vger.kernel.org
Subject: [bug report] nfsd: Implement large extent array support in pNFS
Date: Wed, 16 Jul 2025 14:40:55 -0500 [thread overview]
Message-ID: <9892f785-e9f5-4a29-9ff7-fd89dbf7e474@sabinyo.mountain> (raw)
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 }
next reply other threads:[~2025-07-16 19:40 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-16 19:40 Dan Carpenter [this message]
2025-07-16 20:31 ` [bug report] nfsd: Implement large extent array support in pNFS Chuck Lever
2025-07-17 9:51 ` Sergey Bashirov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=9892f785-e9f5-4a29-9ff7-fd89dbf7e474@sabinyo.mountain \
--to=dan.carpenter@linaro.org \
--cc=linux-nfs@vger.kernel.org \
--cc=sergeybashirov@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox