| Message ID | f28c6b0e2f9510f42ca934f19c4315084e668c21.1560805614.git.christophe.leroy@c-s.fr (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | Additional fixes on Talitos driver | expand |
On Mon, Jun 17, 2019 at 09:15:02PM +0000, Christophe Leroy wrote: > All mapping iterator logic is based on the assumption that sg->offset > is always lower than PAGE_SIZE. > > But there are situations where sg->offset is such that the SG item > is on the second page. In that case sg_copy_to_buffer() fails > properly copying the data into the buffer. One of the reason is > that the data will be outside the kmapped area used to access that > data. > > This patch fixes the issue by adjusting the mapping iterator > offset and pgoffset fields such that offset is always lower than > PAGE_SIZE. > > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> > Fixes: 4225fc8555a9 ("lib/scatterlist: use page iterator in the mapping iterator") > Cc: stable@vger.kernel.org > --- > lib/scatterlist.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) Good catch. > @@ -686,7 +686,12 @@ static bool sg_miter_get_next_page(struct sg_mapping_iter *miter) > sg = miter->piter.sg; > pgoffset = miter->piter.sg_pgoffset; > > - miter->__offset = pgoffset ? 0 : sg->offset; > + offset = pgoffset ? 0 : sg->offset; > + while (offset >= PAGE_SIZE) { > + miter->piter.sg_pgoffset = ++pgoffset; > + offset -= PAGE_SIZE; > + } How about miter->piter.sg_pgoffset += offset >> PAGE_SHIFT; offset &= PAGE_SIZE - 1; Thanks,
Hi, On Thu, Jun 20, 2019 at 02:02:21PM +0800, Herbert Xu wrote: > On Mon, Jun 17, 2019 at 09:15:02PM +0000, Christophe Leroy wrote: > > All mapping iterator logic is based on the assumption that sg->offset > > is always lower than PAGE_SIZE. > > > > But there are situations where sg->offset is such that the SG item > > is on the second page. could you explain how sg->offset becomes >= PAGE_SIZE? --Imre > > In that case sg_copy_to_buffer() fails > > properly copying the data into the buffer. One of the reason is > > that the data will be outside the kmapped area used to access that > > data. > > > > This patch fixes the issue by adjusting the mapping iterator > > offset and pgoffset fields such that offset is always lower than > > PAGE_SIZE. > > > > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> > > Fixes: 4225fc8555a9 ("lib/scatterlist: use page iterator in the mapping iterator") > > Cc: stable@vger.kernel.org > > --- > > lib/scatterlist.c | 9 +++++++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > Good catch. > > > @@ -686,7 +686,12 @@ static bool sg_miter_get_next_page(struct sg_mapping_iter *miter) > > sg = miter->piter.sg; > > pgoffset = miter->piter.sg_pgoffset; > > > > - miter->__offset = pgoffset ? 0 : sg->offset; > > + offset = pgoffset ? 0 : sg->offset; > > + while (offset >= PAGE_SIZE) { > > + miter->piter.sg_pgoffset = ++pgoffset; > > + offset -= PAGE_SIZE; > > + } > > How about > > miter->piter.sg_pgoffset += offset >> PAGE_SHIFT; > offset &= PAGE_SIZE - 1; > > Thanks, > -- > Email: Herbert Xu <herbert@gondor.apana.org.au> > Home Page: http://gondor.apana.org.au/~herbert/ > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Mon, Jun 24, 2019 at 08:35:33PM +0300, Imre Deak wrote: > Hi, > > On Thu, Jun 20, 2019 at 02:02:21PM +0800, Herbert Xu wrote: > > On Mon, Jun 17, 2019 at 09:15:02PM +0000, Christophe Leroy wrote: > > > All mapping iterator logic is based on the assumption that sg->offset > > > is always lower than PAGE_SIZE. > > > > > > But there are situations where sg->offset is such that the SG item > > > is on the second page. > > could you explain how sg->offset becomes >= PAGE_SIZE? The network stack can produce SG list elements that are longer than PAGE_SIZE. If you the iterate over it one page at a time then the offset can exceed PAGE_SIZE. Cheers,
diff --git a/lib/scatterlist.c b/lib/scatterlist.c index 739dc9fe2c55..39f00659898f 100644 --- a/lib/scatterlist.c +++ b/lib/scatterlist.c @@ -678,7 +678,7 @@ static bool sg_miter_get_next_page(struct sg_mapping_iter *miter) { if (!miter->__remaining) { struct scatterlist *sg; - unsigned long pgoffset; + unsigned long pgoffset, offset; if (!__sg_page_iter_next(&miter->piter)) return false; @@ -686,7 +686,12 @@ static bool sg_miter_get_next_page(struct sg_mapping_iter *miter) sg = miter->piter.sg; pgoffset = miter->piter.sg_pgoffset; - miter->__offset = pgoffset ? 0 : sg->offset; + offset = pgoffset ? 0 : sg->offset; + while (offset >= PAGE_SIZE) { + miter->piter.sg_pgoffset = ++pgoffset; + offset -= PAGE_SIZE; + } + miter->__offset = offset; miter->__remaining = sg->offset + sg->length - (pgoffset << PAGE_SHIFT) - miter->__offset; miter->__remaining = min_t(unsigned long, miter->__remaining,
All mapping iterator logic is based on the assumption that sg->offset is always lower than PAGE_SIZE. But there are situations where sg->offset is such that the SG item is on the second page. In that case sg_copy_to_buffer() fails properly copying the data into the buffer. One of the reason is that the data will be outside the kmapped area used to access that data. This patch fixes the issue by adjusting the mapping iterator offset and pgoffset fields such that offset is always lower than PAGE_SIZE. Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> Fixes: 4225fc8555a9 ("lib/scatterlist: use page iterator in the mapping iterator") Cc: stable@vger.kernel.org --- lib/scatterlist.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)