Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

fix return values of seq_read_iter()

Unlike ->read(), ->read_iter() instances *must* return the amount
of data they'd left in iterator. For ->read() returning less than
it has actually copied is a QoI issue; read(fd, unmapped_page - 5, 8)
is allowed to fill all 5 bytes of destination and return 4; it's
not nice to caller, but POSIX allows pretty much anything in such
situation, up to and including a SIGSEGV.

generic_file_splice_read() uses pipe-backed iterator as destination;
there a short copy comes from pipe being full, not from running into
an un{mapped,writable} page in the middle of destination as we
have for iovec-backed iterators read(2) uses. And there we rely
upon the ->read_iter() reporting the actual amount it has left
in destination.

Conversion of a ->read() instance into ->read_iter() has to watch
out for that. If you really need an "all or nothing" kind of
behaviour somewhere, you need to do iov_iter_revert() to prune
the partial copy.

In case of seq_read_iter() we can handle short copy just fine;
the data is in m->buf and next call will fetch it from there.

Fixes: d4d50710a8b4 (seq_file: add seq_read_iter)
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Al Viro 4bbf439b d4d50710

+27 -30
+27 -30
fs/seq_file.c
··· 168 168 ssize_t seq_read_iter(struct kiocb *iocb, struct iov_iter *iter) 169 169 { 170 170 struct seq_file *m = iocb->ki_filp->private_data; 171 - size_t size = iov_iter_count(iter); 172 171 size_t copied = 0; 173 172 size_t n; 174 173 void *p; 175 174 int err = 0; 175 + 176 + if (!iov_iter_count(iter)) 177 + return 0; 176 178 177 179 mutex_lock(&m->lock); 178 180 ··· 208 206 if (!m->buf) 209 207 goto Enomem; 210 208 } 211 - /* if not empty - flush it first */ 209 + // something left in the buffer - copy it out first 212 210 if (m->count) { 213 - n = min(m->count, size); 214 - if (copy_to_iter(m->buf + m->from, n, iter) != n) 215 - goto Efault; 211 + n = copy_to_iter(m->buf + m->from, m->count, iter); 216 212 m->count -= n; 217 213 m->from += n; 218 - size -= n; 219 214 copied += n; 220 - if (!size) 215 + if (m->count) // hadn't managed to copy everything 221 216 goto Done; 222 217 } 223 - /* we need at least one record in buffer */ 218 + // get a non-empty record in the buffer 224 219 m->from = 0; 225 220 p = m->op->start(m, &m->index); 226 221 while (1) { 227 222 err = PTR_ERR(p); 228 - if (!p || IS_ERR(p)) 223 + if (!p || IS_ERR(p)) // EOF or an error 229 224 break; 230 225 err = m->op->show(m, p); 231 - if (err < 0) 226 + if (err < 0) // hard error 232 227 break; 233 - if (unlikely(err)) 228 + if (unlikely(err)) // ->show() says "skip it" 234 229 m->count = 0; 235 - if (unlikely(!m->count)) { 230 + if (unlikely(!m->count)) { // empty record 236 231 p = m->op->next(m, p, &m->index); 237 232 continue; 238 233 } 239 - if (m->count < m->size) 234 + if (!seq_has_overflowed(m)) // got it 240 235 goto Fill; 236 + // need a bigger buffer 241 237 m->op->stop(m, p); 242 238 kvfree(m->buf); 243 239 m->count = 0; ··· 244 244 goto Enomem; 245 245 p = m->op->start(m, &m->index); 246 246 } 247 + // EOF or an error 247 248 m->op->stop(m, p); 248 249 m->count = 0; 249 250 goto Done; 250 251 Fill: 251 - /* they want more? let's try to get some more */ 252 + // one non-empty record is in the buffer; if they want more, 253 + // try to fit more in, but in any case we need to advance 254 + // the iterator once for every record shown. 252 255 while (1) { 253 256 size_t offs = m->count; 254 257 loff_t pos = m->index; ··· 262 259 m->op->next); 263 260 m->index++; 264 261 } 265 - if (!p || IS_ERR(p)) { 266 - err = PTR_ERR(p); 262 + if (!p || IS_ERR(p)) // no next record for us 267 263 break; 268 - } 269 - if (m->count >= size) 264 + if (m->count >= iov_iter_count(iter)) 270 265 break; 271 266 err = m->op->show(m, p); 272 - if (seq_has_overflowed(m) || err) { 267 + if (err > 0) { // ->show() says "skip it" 273 268 m->count = offs; 274 - if (likely(err <= 0)) 275 - break; 269 + } else if (err || seq_has_overflowed(m)) { 270 + m->count = offs; 271 + break; 276 272 } 277 273 } 278 274 m->op->stop(m, p); 279 - n = min(m->count, size); 280 - if (copy_to_iter(m->buf, n, iter) != n) 281 - goto Efault; 275 + n = copy_to_iter(m->buf, m->count, iter); 282 276 copied += n; 283 277 m->count -= n; 284 278 m->from = n; 285 279 Done: 286 - if (!copied) 287 - copied = err; 288 - else { 280 + if (unlikely(!copied)) { 281 + copied = m->count ? -EFAULT : err; 282 + } else { 289 283 iocb->ki_pos += copied; 290 284 m->read_pos += copied; 291 285 } ··· 290 290 return copied; 291 291 Enomem: 292 292 err = -ENOMEM; 293 - goto Done; 294 - Efault: 295 - err = -EFAULT; 296 293 goto Done; 297 294 } 298 295 EXPORT_SYMBOL(seq_read_iter);