On 05/01/2023 15:36, Jason Gunthorpe wrote:
On Thu, Jan 05, 2023 at 01:23:39PM +0200, Yishai Hadas wrote:
When sg_alloc_append_table_from_pages() calls to pages_are_mergeable()
in its 'sgt_append->prv' flow to check whether it can merge contiguous
pages into the last SG, it passes the page arguments in the wrong order.
The first parameter should be the next candidate page to be merged to
the last page and not the opposite.
The current code leads to a corrupted SG which resulted in OOPs and
unexpected errors when non-contiguous pages are merged wrongly.
Fix to pass the page parameters in the right order.
Fixes: 1567b49d1a40 ("lib/scatterlist: add check when merging zone device pages")
Signed-off-by: Yishai Hadas <yishaih@xxxxxxxxxx>
---
lib/scatterlist.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
Thanks Jason
Also, I'm looking more closely at '156 and this is not right either:
- unsigned long paddr =
- (page_to_pfn(sg_page(sgt_append->prv)) * PAGE_SIZE +
- sgt_append->prv->offset + sgt_append->prv->length) /
- PAGE_SIZE;
-
- while (n_pages && page_to_pfn(pages[0]) == paddr) {
+ last_pg = sg_page(sgt_append->prv);
+ while (n_pages && pages_are_mergeable(last_pg, pages[0])) {
This change will break things like multi-page combining, sub page
scenarios and maybe more.
The contiguity test here has to be done a phys, it should go back to
struct page to check if the pgmap is OK.
Can you fix it as well?
Yes, I have locally some candidate patch as you asked, on top of this one.
I would like to run some extra testing on, then may send it.
Yishai
