| Message ID | 1967121.1714034372@warthog.procyon.org.uk (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 6a30653b604aaad1bf0f2e74b068ceb8b6fc7aea |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] Fix a potential infinite loop in extract_user_to_sg() | expand |
On Thu, 25 Apr 2024 09:39:32 +0100 David Howells wrote: > Fix extract_user_to_sg() so that it will break out of the loop if > iov_iter_extract_pages() returns 0 rather than looping around forever. Is "goto fail" the right way to break out here? My intuition would be "break". On a quick read it seems like res = 0 may occur if we run out of iterator, is passing maxsize > iter->count illegal?
Jakub Kicinski <kuba@kernel.org> wrote: > On Thu, 25 Apr 2024 09:39:32 +0100 David Howells wrote: > > Fix extract_user_to_sg() so that it will break out of the loop if > > iov_iter_extract_pages() returns 0 rather than looping around forever. > > Is "goto fail" the right way to break out here? > My intuition would be "break". > > On a quick read it seems like res = 0 may occur if we run out of > iterator, is passing maxsize > iter->count illegal? I would say that you're not allowed to ask for more than is in the iterator. In a number of places this is called, it's a clear failure if you can't get that the requested amount out of it - for example, if we're building a cifs message and have set all the fields in the header and are trying to encrypt the message. David
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Thu, 25 Apr 2024 09:39:32 +0100 you wrote: > Fix extract_user_to_sg() so that it will break out of the loop if > iov_iter_extract_pages() returns 0 rather than looping around forever. > > [Note that I've included two fixes lines as the function got moved to a > different file and renamed] > > Fixes: 85dd2c8ff368 ("netfs: Add a function to extract a UBUF or IOVEC into a BVEC iterator") > Fixes: f5f82cd18732 ("Move netfs_extract_iter_to_sg() to lib/scatterlist.c") > Signed-off-by: David Howells <dhowells@redhat.com> > cc: Jeff Layton <jlayton@kernel.org> > cc: Steve French <sfrench@samba.org> > cc: Herbert Xu <herbert@gondor.apana.org.au> > cc: "David S. Miller" <davem@davemloft.net> > cc: Eric Dumazet <edumazet@google.com> > cc: Jakub Kicinski <kuba@kernel.org> > cc: Paolo Abeni <pabeni@redhat.com> > cc: netfs@lists.linux.dev > cc: linux-crypto@vger.kernel.org > cc: linux-cifs@vger.kernel.org > cc: linux-fsdevel@vger.kernel.org > cc: netdev@vger.kernel.org > > [...] Here is the summary with links: - [net] Fix a potential infinite loop in extract_user_to_sg() https://git.kernel.org/netdev/net/c/6a30653b604a You are awesome, thank you!
diff --git a/lib/scatterlist.c b/lib/scatterlist.c index 68b45c82c37a..7bc2220fea80 100644 --- a/lib/scatterlist.c +++ b/lib/scatterlist.c @@ -1124,7 +1124,7 @@ static ssize_t extract_user_to_sg(struct iov_iter *iter, do { res = iov_iter_extract_pages(iter, &pages, maxsize, sg_max, extraction_flags, &off); - if (res < 0) + if (res <= 0) goto failed; len = res;
Fix extract_user_to_sg() so that it will break out of the loop if iov_iter_extract_pages() returns 0 rather than looping around forever. [Note that I've included two fixes lines as the function got moved to a different file and renamed] Fixes: 85dd2c8ff368 ("netfs: Add a function to extract a UBUF or IOVEC into a BVEC iterator") Fixes: f5f82cd18732 ("Move netfs_extract_iter_to_sg() to lib/scatterlist.c") Signed-off-by: David Howells <dhowells@redhat.com> cc: Jeff Layton <jlayton@kernel.org> cc: Steve French <sfrench@samba.org> cc: Herbert Xu <herbert@gondor.apana.org.au> cc: "David S. Miller" <davem@davemloft.net> cc: Eric Dumazet <edumazet@google.com> cc: Jakub Kicinski <kuba@kernel.org> cc: Paolo Abeni <pabeni@redhat.com> cc: netfs@lists.linux.dev cc: linux-crypto@vger.kernel.org cc: linux-cifs@vger.kernel.org cc: linux-fsdevel@vger.kernel.org cc: netdev@vger.kernel.org --- lib/scatterlist.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)