net/rds: make copy_page_from_iter and copy_page_to_iter stay within page boundaries (WORKAROUND!)

The original UEK-4.1 code allocated multiple pages via rds_page_remainder_alloc
up to RDS_MAX_FRAG_SIZE (16kiB), as introduced in commit 23f90cc.
Before commit 23f90cc, as well as the upstream-Linux version allocates
single pages only.
Since these multiple pages are allocated in a physically contiguous area (alloc_pages),
neither the review nor tests revealed the fact that copy_page_from_iter does not
support crossing page-boundaries:
The code wasn't tested on platforms where HIGHMEM pages would have to be
mapped into the kernel (kmap) explicitly, such as i386 (32bit) anymore.
On those platforms it most likely would have just crashed!
Only when commit 72e809e came along, that
introduced an explicit check for not crossing page boundaries in iov_iter.c
came along, did this problem become obvious.
There's also the issue with this approach of allocating physically contiguous 16KiB areas:
it does not back down and try to allocate smaller orders (down to 0), in case allocation failed.
Also, copy_page_from_iter may not be the only function mapping HIGHMEM pages,
hence unable to cross page boundaries (as kmap maps a single page).
So the idea came up to simply revert commit 23f90cc, since it was also just introduced
on behalf of the PSIF project, which is now defunct.
Unfortunately and unexpectedly the old code (of allocating single pages) no longer works!
That leads to the obvious conclusion:
Some code change that happened after commit 23f90cc in UEK makes either an
explicit assumption (without or with hidden note), or an implicit assumption (e.g. a one-off bug)
about the presence of this multi-page allocation.

Orabug: 27222215
Orabug: 27364391

Signed-off-by: Gerd Rausch <[email protected]>

Reviewed-by: Sudhakar Dindukurti <[email protected]>
Signed-off-by: Somasundaram Krishnasamy <[email protected]>

Author: gerd-rausch

net/rds/ib_recv.c

@@ -785,40 +785,40 @@ int rds_ib_inc_copy_to_user(struct rds_incoming *inc, struct iov_iter *to)
 	struct rds_ib_connection *ic = inc->i_conn->c_transport_data;
 	struct rds_ib_incoming *ibinc;
 	struct rds_page_frag *frag;
-	unsigned long to_copy;
-	unsigned long frag_off = 0;
-	int copied = 0;
-	int ret;
-	u32 len;
+	size_t len, saved_len, to_copy, copied, offset, chunk;
 
 	ibinc = container_of(inc, struct rds_ib_incoming, ii_inc);
 	frag = list_entry(ibinc->ii_frags.next, struct rds_page_frag, f_item);
+
 	len = be32_to_cpu(inc->i_hdr.h_len);
+	if (iov_iter_count(to) < len)
+		len = iov_iter_count(to);
 
-	while (iov_iter_count(to) && copied < len) {
-		if (frag_off == ic->i_frag_sz) {
-			frag = list_entry(frag->f_item.next,
-					  struct rds_page_frag, f_item);
-			frag_off = 0;
-		}
-		to_copy = min_t(unsigned long, iov_iter_count(to),
-				ic->i_frag_sz - frag_off);
-		to_copy = min_t(unsigned long, to_copy, len - copied);
+	saved_len = len;
+
+	while (len > 0) {
+		to_copy = len < ic->i_frag_sz ? len : ic->i_frag_sz;
 
-		/* XXX needs + offset for multiple recvs per page */
 		rds_stats_add(s_copy_to_user, to_copy);
-		ret = copy_page_to_iter(sg_page(&frag->f_sg),
-					frag->f_sg.offset + frag_off,
-					to_copy,
-					to);
-		if (ret != to_copy)
-			return -EFAULT;
 
-		frag_off += to_copy;
-		copied += to_copy;
+		for (copied = 0; copied < to_copy; copied += chunk) {
+			offset = frag->f_sg.offset + copied;
+			chunk = offset < PAGE_SIZE ? PAGE_SIZE - offset : PAGE_SIZE;
+			if (copied + chunk > to_copy)
+				chunk = to_copy - copied;
+
+			if (copy_page_to_iter(sg_page(&frag->f_sg) + (offset >> PAGE_SHIFT),
+					      offset & ~PAGE_MASK,
+					      chunk, to) != chunk)
+				return -EFAULT;
+		}
+
+		frag = list_entry(frag->f_item.next,
+				  struct rds_page_frag, f_item);
+		len -= copied;
 	}
 
-	return copied;
+	return saved_len;
 }
 
 /* ic starts out kzalloc()ed */

net/rds/message.c

@@ -295,9 +295,8 @@ struct scatterlist *rds_message_alloc_sgs(struct rds_message *rm, int nents)
 int rds_message_copy_from_user(struct rds_message *rm, struct iov_iter *from,
 			       gfp_t gfp, bool large_page)
 {
-	unsigned long to_copy, nbytes;
-	unsigned long sg_off;
 	struct scatterlist *sg;
+	size_t to_copy, copied, offset, chunk;
 	int ret = 0;
 
 	rm->m_inc.i_hdr.h_len = cpu_to_be32(iov_iter_count(from));
@@ -306,7 +305,6 @@ int rds_message_copy_from_user(struct rds_message *rm, struct iov_iter *from,
 	 * now allocate and copy in the data payload.
 	 */
 	sg = rm->data.op_sg;
-	sg_off = 0; /* Dear gcc, sg->page will be null from kzalloc. */
 
 	while (iov_iter_count(from)) {
 		if (!sg_page(sg)) {
@@ -318,65 +316,69 @@ int rds_message_copy_from_user(struct rds_message *rm, struct iov_iter *from,
 			if (ret)
 				return ret;
 			rm->data.op_nents++;
-			sg_off = 0;
 		}
 
-		to_copy = min_t(unsigned long, iov_iter_count(from),
-				sg->length - sg_off);
+		to_copy = iov_iter_count(from);
+		if (to_copy > sg->length)
+			to_copy = sg->length;
 
 		rds_stats_add(s_copy_from_user, to_copy);
-		nbytes = copy_page_from_iter(sg_page(sg), sg->offset + sg_off,
-					     to_copy, from);
-		if (nbytes != to_copy)
-			return -EFAULT;
 
-		sg_off += to_copy;
+		for (copied = 0; copied < to_copy; copied += chunk) {
+			offset = sg->offset + copied;
+			chunk = offset < PAGE_SIZE ? PAGE_SIZE - offset : PAGE_SIZE;
+			if (copied + chunk > to_copy)
+				chunk = to_copy - copied;
 
-		if (sg_off == sg->length)
-			sg++;
+			if (copy_page_from_iter(sg_page(sg) + (offset >> PAGE_SHIFT),
+						offset & ~PAGE_MASK,
+						chunk, from) != chunk)
+				return -EFAULT;
+		}
+
+		sg++;
 	}
 
-	return ret;
+	return 0;
 }
 
 int rds_message_inc_copy_to_user(struct rds_incoming *inc, struct iov_iter *to)
 {
 	struct rds_message *rm;
 	struct scatterlist *sg;
-	unsigned long to_copy;
-	unsigned long vec_off;
-	int copied;
-	int ret;
-	u32 len;
+	size_t len, saved_len, to_copy, copied, offset, chunk;
 
 	rm = container_of(inc, struct rds_message, m_inc);
 	len = be32_to_cpu(rm->m_inc.i_hdr.h_len);
+	if (iov_iter_count(to) < len)
+		len = iov_iter_count(to);
 
 	sg = rm->data.op_sg;
-	vec_off = 0;
-	copied = 0;
 
-	while (iov_iter_count(to) && copied < len) {
-		to_copy = min_t(unsigned long, iov_iter_count(to),
-				sg->length - vec_off);
-		to_copy = min_t(unsigned long, to_copy, len - copied);
+	saved_len = len;
+
+	while (len > 0) {
+		to_copy = len < sg->length ? len : sg->length;
 
 		rds_stats_add(s_copy_to_user, to_copy);
-		ret = copy_page_to_iter(sg_page(sg), sg->offset + vec_off,
-					to_copy, to);
-		if (ret != to_copy)
-			return -EFAULT;
 
-		vec_off += to_copy;
-		copied += to_copy;
+		for (copied = 0; copied < to_copy; copied += chunk) {
+			offset = sg->offset + copied;
+			chunk = offset < PAGE_SIZE ? PAGE_SIZE - offset : PAGE_SIZE;
+			if (copied + chunk > to_copy)
+				chunk = to_copy - copied;
 
-		if (vec_off == sg->length) {
-			vec_off = 0;
-			sg++;
+			if (copy_page_to_iter(sg_page(sg) + (offset >> PAGE_SHIFT),
+					      offset & ~PAGE_MASK,
+					      chunk, to) != chunk)
+				return -EFAULT;
 		}
+
+		sg++;
+		len -= copied;
 	}
 
-	return copied;
+	return saved_len;
 }
 
 /*