[PATCH v8 0/7] Introduce sendpage_ok() to detect misused sendpage in network related drivers

[Coly Li]

This series was original by a bug fix in nvme-over-tcp driver which only
checked whether a page was allocated from slab allcoator, but forgot to
check its page_count: The page handled by sendpage should be neither a
Slab page nor 0 page_count page.

As Sagi Grimberg suggested, the original fix is refind to a more common
inline routine:
static inline bool sendpage_ok(struct page *page)
{
return (!PageSlab(page) && page_count(page) >= 1);
}
If sendpage_ok() returns true, the checking page can be handled by the
concrete zero-copy sendpage method in network layer.

The v8 series has 7 patches,
- The 1st patch in this series introduces sendpage_ok() in header file
include/linux/net.h.
- The 2nd patch adds WARN_ONCE() for improper zero-copy send in
kernel_sendpage().
- The 3rd patch fixes the page checking issue in nvme-over-tcp driver.
- The 4th patch adds page_count check by using sendpage_ok() in
do_tcp_sendpages() as Eric Dumazet suggested.
- The 5th and 6th patches just replace existing open coded checks with
the inline sendpage_ok() routine.

Coly Li

Cc: Chaitanya Kulkarni <chaitanya...@wdc.com>
Cc: Chris Leech <cle...@redhat.com>
Cc: Christoph Hellwig <h...@lst.de>
Cc: Cong Wang <amw...@redhat.com>
Cc: David S. Miller <da...@davemloft.net>
Cc: Eric Dumazet <eric.d...@gmail.com>
Cc: Hannes Reinecke <ha...@suse.de>
Cc: Ilya Dryomov <idry...@gmail.com>
Cc: Jan Kara <ja...@suse.com>
Cc: Jeff Layton <jla...@kernel.org>
Cc: Jens Axboe <ax...@kernel.dk>
Cc: Lee Duncan <ldu...@suse.com>
Cc: Mike Christie <mich...@cs.wisc.edu>
Cc: Mikhail Skorzhinskii <mskorz...@solarflare.com>
Cc: Philipp Reisner <philipp...@linbit.com>
Cc: Sagi Grimberg <sa...@grimberg.me>
Cc: Vasily Averin <v...@virtuozzo.com>
Cc: Vlastimil Babka <vba...@suse.com>
---
Changelog:
v8: add WARN_ONCE() in kernel_sendpage() as Christoph suggested.
v7: remove outer brackets from the return line of sendpage_ok() as
Eric Dumazet suggested.
v6: fix page check in do_tcp_sendpages(), as Eric Dumazet suggested.
replace other open coded checks with sendpage_ok() in libceph,
iscsi drivers.
v5, include linux/mm.h in include/linux/net.h
v4, change sendpage_ok() as an inline helper, and post it as
separate patch, as Christoph Hellwig suggested.
v3, introduce a more common sendpage_ok() as Sagi Grimberg suggested.
v2, fix typo in patch subject
v1, the initial version.

Coly Li (7):
net: introduce helper sendpage_ok() in include/linux/net.h
net: add WARN_ONCE in kernel_sendpage() for improper zero-copy send
nvme-tcp: check page by sendpage_ok() before calling kernel_sendpage()
tcp: use sendpage_ok() to detect misused .sendpage
drbd: code cleanup by using sendpage_ok() to check page for
kernel_sendpage()
scsi: libiscsi: use sendpage_ok() in iscsi_tcp_segment_map()
libceph: use sendpage_ok() in ceph_tcp_sendpage()

drivers/block/drbd/drbd_main.c | 2 +-
drivers/nvme/host/tcp.c | 7 +++----
drivers/scsi/libiscsi_tcp.c | 2 +-
include/linux/net.h | 16 ++++++++++++++++
net/ceph/messenger.c | 2 +-
net/ipv4/tcp.c | 3 ++-
net/socket.c | 6 ++++--
7 files changed, 28 insertions(+), 10 deletions(-)

--
2.26.2

[Coly Li]

The original problem was from nvme-over-tcp code, who mistakenly uses
kernel_sendpage() to send pages allocated by __get_free_pages() without
__GFP_COMP flag. Such pages don't have refcount (page_count is 0) on
tail pages, sending them by kernel_sendpage() may trigger a kernel panic
from a corrupted kernel heap, because these pages are incorrectly freed
in network stack as page_count 0 pages.

This patch introduces a helper sendpage_ok(), it returns true if the
checking page,
- is not slab page: PageSlab(page) is false.
- has page refcount: page_count(page) is not zero

All drivers who want to send page to remote end by kernel_sendpage()
may use this helper to check whether the page is OK. If the helper does
not return true, the driver should try other non sendpage method (e.g.
sock_no_sendpage()) to handle the page.

Signed-off-by: Coly Li <col...@suse.de>
Cc: Chaitanya Kulkarni <chaitanya...@wdc.com>
Cc: Christoph Hellwig <h...@lst.de>
Cc: Hannes Reinecke <ha...@suse.de>
Cc: Jan Kara <ja...@suse.com>
Cc: Jens Axboe <ax...@kernel.dk>

Cc: Mikhail Skorzhinskii <mskorz...@solarflare.com>
Cc: Philipp Reisner <philipp...@linbit.com>
Cc: Sagi Grimberg <sa...@grimberg.me>

Cc: Vlastimil Babka <vba...@suse.com>
Cc: sta...@vger.kernel.org
---
include/linux/net.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/include/linux/net.h b/include/linux/net.h
index d48ff1180879..05db8690f67e 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -21,6 +21,7 @@
#include <linux/rcupdate.h>
#include <linux/once.h>
#include <linux/fs.h>
+#include <linux/mm.h>
#include <linux/sockptr.h>

#include <uapi/linux/net.h>
@@ -286,6 +287,21 @@ do { \
#define net_get_random_once_wait(buf, nbytes) \
get_random_once_wait((buf), (nbytes))

+/*
+ * E.g. XFS meta- & log-data is in slab pages, or bcache meta
+ * data pages, or other high order pages allocated by
+ * __get_free_pages() without __GFP_COMP, which have a page_count
+ * of 0 and/or have PageSlab() set. We cannot use send_page for
+ * those, as that does get_page(); put_page(); and would cause
+ * either a VM_BUG directly, or __page_cache_release a page that
+ * would actually still be referenced by someone, leading to some
+ * obscure delayed Oops somewhere else.
+ */
+static inline bool sendpage_ok(struct page *page)
+{
+ return !PageSlab(page) && page_count(page) >= 1;
+}
+
int kernel_sendmsg(struct socket *sock, struct msghdr *msg, struct kvec *vec,
size_t num, size_t len);
int kernel_sendmsg_locked(struct sock *sk, struct msghdr *msg,
--
2.26.2

[Coly Li]

If a page sent into kernel_sendpage() is a slab page or it doesn't have
ref_count, this page is improper to send by the zero copy sendpage()
method. Otherwise such page might be unexpected released in network code
path and causes impredictable panic due to kernel memory management data
structure corruption.

This path adds a WARN_ON() on the sending page before sends it into the
concrete zero-copy sendpage() method, if the page is improper for the
zero-copy sendpage() method, a warning message can be observed before
the consequential unpredictable kernel panic.

This patch does not change existing kernel_sendpage() behavior for the
improper page zero-copy send, it just provides hint warning message for
following potential panic due the kernel memory heap corruption.

Signed-off-by: Coly Li <col...@suse.de>
Cc: Cong Wang <amw...@redhat.com>
Cc: Christoph Hellwig <h...@lst.de>

Cc: David S. Miller <da...@davemloft.net>

Cc: Sridhar Samudrala <s...@us.ibm.com>
---
net/socket.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index 0c0144604f81..771456a1d947 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -3638,9 +3638,11 @@ EXPORT_SYMBOL(kernel_getpeername);
int kernel_sendpage(struct socket *sock, struct page *page, int offset,
size_t size, int flags)
{
- if (sock->ops->sendpage)
+ if (sock->ops->sendpage) {
+ /* Warn in case the improper page to zero-copy send */
+ WARN_ONCE(!sendpage_ok(page));
return sock->ops->sendpage(sock, page, offset, size, flags);
-
+ }
return sock_no_sendpage(sock, page, offset, size, flags);
}
EXPORT_SYMBOL(kernel_sendpage);
--
2.26.2

[Coly Li]

Currently nvme_tcp_try_send_data() doesn't use kernel_sendpage() to
send slab pages. But for pages allocated by __get_free_pages() without
__GFP_COMP, which also have refcount as 0, they are still sent by
kernel_sendpage() to remote end, this is problematic.

The new introduced helper sendpage_ok() checks both PageSlab tag and
page_count counter, and returns true if the checking page is OK to be
sent by kernel_sendpage().

This patch fixes the page checking issue of nvme_tcp_try_send_data()
with sendpage_ok(). If sendpage_ok() returns true, send this page by
kernel_sendpage(), otherwise use sock_no_sendpage to handle this page.

Signed-off-by: Coly Li <col...@suse.de>
Cc: Chaitanya Kulkarni <chaitanya...@wdc.com>
Cc: Christoph Hellwig <h...@lst.de>
Cc: Hannes Reinecke <ha...@suse.de>
Cc: Jan Kara <ja...@suse.com>
Cc: Jens Axboe <ax...@kernel.dk>

Cc: Mikhail Skorzhinskii <mskorz...@solarflare.com>
Cc: Philipp Reisner <philipp...@linbit.com>
Cc: Sagi Grimberg <sa...@grimberg.me>

Cc: Vlastimil Babka <vba...@suse.com>
Cc: sta...@vger.kernel.org
---

drivers/nvme/host/tcp.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 8f4f29f18b8c..d6a3e1487354 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -913,12 +913,11 @@ static int nvme_tcp_try_send_data(struct nvme_tcp_request *req)
else
flags |= MSG_MORE | MSG_SENDPAGE_NOTLAST;

- /* can't zcopy slab pages */
- if (unlikely(PageSlab(page))) {
- ret = sock_no_sendpage(queue->sock, page, offset, len,
+ if (sendpage_ok(page)) {
+ ret = kernel_sendpage(queue->sock, page, offset, len,
flags);
} else {
- ret = kernel_sendpage(queue->sock, page, offset, len,
+ ret = sock_no_sendpage(queue->sock, page, offset, len,
flags);
}
if (ret <= 0)
--
2.26.2

[Coly Li]

commit a10674bf2406 ("tcp: detecting the misuse of .sendpage for Slab
objects") adds the checks for Slab pages, but the pages don't have
page_count are still missing from the check.

Network layer's sendpage method is not designed to send page_count 0
pages neither, therefore both PageSlab() and page_count() should be
both checked for the sending page. This is exactly what sendpage_ok()
does.

This patch uses sendpage_ok() in do_tcp_sendpages() to detect misused
.sendpage, to make the code more robust.

Fixes: a10674bf2406 ("tcp: detecting the misuse of .sendpage for Slab objects")
Suggested-by: Eric Dumazet <eric.d...@gmail.com>
Signed-off-by: Coly Li <col...@suse.de>
Cc: Vasily Averin <v...@virtuozzo.com>

Cc: David S. Miller <da...@davemloft.net>

Cc: sta...@vger.kernel.org
---
net/ipv4/tcp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 31f3b858db81..2135ee7c806d 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -970,7 +970,8 @@ ssize_t do_tcp_sendpages(struct sock *sk, struct page *page, int offset,
long timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);

if (IS_ENABLED(CONFIG_DEBUG_VM) &&
- WARN_ONCE(PageSlab(page), "page must not be a Slab one"))
+ WARN_ONCE(!sendpage_ok(page),
+ "page must not be a Slab one and have page_count > 0"))
return -EINVAL;

/* Wait for a connection to finish. One exception is TCP Fast Open
--
2.26.2

[Coly Li]

In _drbd_send_page() a page is checked by following code before sending
it by kernel_sendpage(),
(page_count(page) < 1) || PageSlab(page)
If the check is true, this page won't be send by kernel_sendpage() and
handled by sock_no_sendpage().

This kind of check is exactly what macro sendpage_ok() does, which is
introduced into include/linux/net.h to solve a similar send page issue
in nvme-tcp code.

This patch uses macro sendpage_ok() to replace the open coded checks to
page type and refcount in _drbd_send_page(), as a code cleanup.

Signed-off-by: Coly Li <col...@suse.de>

Cc: Philipp Reisner <philipp...@linbit.com>
Cc: Sagi Grimberg <sa...@grimberg.me>

---
drivers/block/drbd/drbd_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/block/drbd/drbd_main.c b/drivers/block/drbd/drbd_main.c
index 04b6bde9419d..573dbf6f0c31 100644
--- a/drivers/block/drbd/drbd_main.c
+++ b/drivers/block/drbd/drbd_main.c
@@ -1553,7 +1553,7 @@ static int _drbd_send_page(struct drbd_peer_device *peer_device, struct page *pa
* put_page(); and would cause either a VM_BUG directly, or
* __page_cache_release a page that would actually still be referenced
* by someone, leading to some obscure delayed Oops somewhere else. */
- if (drbd_disable_sendpage || (page_count(page) < 1) || PageSlab(page))
+ if (drbd_disable_sendpage || !sendpage_ok(page))
return _drbd_no_send_page(peer_device, page, offset, size, msg_flags);

msg_flags |= MSG_NOSIGNAL;
--
2.26.2

[Coly Li]

In iscsci driver, iscsi_tcp_segment_map() uses the following code to
check whether the page should or not be handled by sendpage:
if (!recv && page_count(sg_page(sg)) >= 1 && !PageSlab(sg_page(sg)))

The "page_count(sg_page(sg)) >= 1 && !PageSlab(sg_page(sg)" part is to
make sure the page can be sent to network layer's zero copy path. This
part is exactly what sendpage_ok() does.

This patch uses use sendpage_ok() in iscsi_tcp_segment_map() to replace
the original open coded checks.

Signed-off-by: Coly Li <col...@suse.de>
Cc: Vasily Averin <v...@virtuozzo.com>
Cc: Cong Wang <amw...@redhat.com>
Cc: Mike Christie <mich...@cs.wisc.edu>
Cc: Lee Duncan <ldu...@suse.com>

Cc: Chris Leech <cle...@redhat.com>
Cc: Christoph Hellwig <h...@lst.de>

Cc: Hannes Reinecke <ha...@suse.de>
---
drivers/scsi/libiscsi_tcp.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/libiscsi_tcp.c b/drivers/scsi/libiscsi_tcp.c
index 37e5d4e48c2f..83f14b2c8804 100644
--- a/drivers/scsi/libiscsi_tcp.c
+++ b/drivers/scsi/libiscsi_tcp.c
@@ -128,7 +128,7 @@ static void iscsi_tcp_segment_map(struct iscsi_segment *segment, int recv)
* coalescing neighboring slab objects into a single frag which
* triggers one of hardened usercopy checks.
*/
- if (!recv && page_count(sg_page(sg)) >= 1 && !PageSlab(sg_page(sg)))
+ if (!recv && sendpage_ok(sg_page(sg)))
return;

if (recv) {
--
2.26.2

[Coly Li]

In libceph, ceph_tcp_sendpage() does the following checks before handle
the page by network layer's zero copy sendpage method,
if (page_count(page) >= 1 && !PageSlab(page))

This check is exactly what sendpage_ok() does. This patch replace the
open coded checks by sendpage_ok() as a code cleanup.

Signed-off-by: Coly Li <col...@suse.de>
Cc: Ilya Dryomov <idry...@gmail.com>
Cc: Jeff Layton <jla...@kernel.org>
---
net/ceph/messenger.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index bdfd66ba3843..d4d7a0e52491 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -575,7 +575,7 @@ static int ceph_tcp_sendpage(struct socket *sock, struct page *page,

* coalescing neighboring slab objects into a single frag which
* triggers one of hardened usercopy checks.
*/

- if (page_count(page) >= 1 && !PageSlab(page))
+ if (sendpage_ok(page))
sendpage = sock->ops->sendpage;
else
sendpage = sock_no_sendpage;
--
2.26.2

[Greg KH]

Do you have one extra ' ' after "return" there?

And this feels like a mm thing, why put it in net.h and not mm.h?

thanks,

greg k-h

[Martin K. Petersen]

Coly,

> In iscsci driver, iscsi_tcp_segment_map() uses the following code to
> check whether the page should or not be handled by sendpage:
> if (!recv && page_count(sg_page(sg)) >= 1 && !PageSlab(sg_page(sg)))
>
> The "page_count(sg_page(sg)) >= 1 && !PageSlab(sg_page(sg)" part is to
> make sure the page can be sent to network layer's zero copy path. This
> part is exactly what sendpage_ok() does.
>
> This patch uses use sendpage_ok() in iscsi_tcp_segment_map() to replace
> the original open coded checks.

Looks fine to me.

Acked-by: Martin K. Petersen <martin....@oracle.com>

--
Martin K. Petersen Oracle Linux Engineering

[Coly Li]

It should be fixed in next version.

>
> And this feels like a mm thing, why put it in net.h and not mm.h?

This check is specific for kernel_sendpage(), so I want to place it
closer to where kernel_sendpage() is declared.

And indeed there was similar discussion about why this helper is not in
mm code in v5 series. Christoph supported to place sendpage_ok() in
net.h, an uncompleted piece of his opinion was "It is not a mm bug, it
is a networking quirk."

Thanks.

Coly Li

[Jeff Layton]

Looks like a reasonable change to make. Assuming that there is no
objection to the new helper:

Acked-by: Jeff Layton <jla...@kernel.org>

[Greg KH]

Ah, nevermind then, sorry for the noise :)

greg k-h

[Coly Li]

This series was original by a bug fix in nvme-over-tcp driver which only
checked whether a page was allocated from slab allcoator, but forgot to
check its page_count: The page handled by sendpage should be neither a
Slab page nor 0 page_count page.

As Sagi Grimberg suggested, the original fix is refind to a more common
inline routine:

static inline bool sendpage_ok(struct page *page)

{
return (!PageSlab(page) && page_count(page) >= 1);
}
If sendpage_ok() returns true, the checking page can be handled by the
concrete zero-copy sendpage method in network layer.

The v9 series has 7 patches, no change from v8 series,

- The 1st patch in this series introduces sendpage_ok() in header file
include/linux/net.h.
- The 2nd patch adds WARN_ONCE() for improper zero-copy send in
kernel_sendpage().
- The 3rd patch fixes the page checking issue in nvme-over-tcp driver.
- The 4th patch adds page_count check by using sendpage_ok() in
do_tcp_sendpages() as Eric Dumazet suggested.
- The 5th and 6th patches just replace existing open coded checks with
the inline sendpage_ok() routine.

Coly Li

Cc: Chaitanya Kulkarni <chaitanya...@wdc.com>
Cc: Chris Leech <cle...@redhat.com>
Cc: Christoph Hellwig <h...@lst.de>
Cc: Cong Wang <amw...@redhat.com>

Cc: David S. Miller <da...@davemloft.net>

Cc: Eric Dumazet <eric.d...@gmail.com>
Cc: Hannes Reinecke <ha...@suse.de>
Cc: Ilya Dryomov <idry...@gmail.com>
Cc: Jan Kara <ja...@suse.com>
Cc: Jeff Layton <jla...@kernel.org>
Cc: Jens Axboe <ax...@kernel.dk>

Cc: Lee Duncan <ldu...@suse.com>
Cc: Mike Christie <mich...@cs.wisc.edu>

Cc: Mikhail Skorzhinskii <mskorz...@solarflare.com>
Cc: Philipp Reisner <philipp...@linbit.com>
Cc: Sagi Grimberg <sa...@grimberg.me>

Cc: Vasily Averin <v...@virtuozzo.com>
Cc: Vlastimil Babka <vba...@suse.com>
---
Changelog:

v9, fix a typo pointed out by Greg KH.
add Acked-by tags from Martin K. Petersen and Ilya Dryomov.

[Coly Li]

The original problem was from nvme-over-tcp code, who mistakenly uses
kernel_sendpage() to send pages allocated by __get_free_pages() without
__GFP_COMP flag. Such pages don't have refcount (page_count is 0) on
tail pages, sending them by kernel_sendpage() may trigger a kernel panic
from a corrupted kernel heap, because these pages are incorrectly freed
in network stack as page_count 0 pages.

This patch introduces a helper sendpage_ok(), it returns true if the
checking page,
- is not slab page: PageSlab(page) is false.
- has page refcount: page_count(page) is not zero

All drivers who want to send page to remote end by kernel_sendpage()
may use this helper to check whether the page is OK. If the helper does
not return true, the driver should try other non sendpage method (e.g.
sock_no_sendpage()) to handle the page.

Signed-off-by: Coly Li <col...@suse.de>

Cc: Chaitanya Kulkarni <chaitanya...@wdc.com>
Cc: Christoph Hellwig <h...@lst.de>
Cc: Hannes Reinecke <ha...@suse.de>
Cc: Jan Kara <ja...@suse.com>
Cc: Jens Axboe <ax...@kernel.dk>

Cc: Mikhail Skorzhinskii <mskorz...@solarflare.com>
Cc: Philipp Reisner <philipp...@linbit.com>
Cc: Sagi Grimberg <sa...@grimberg.me>

Cc: Vlastimil Babka <vba...@suse.com>
Cc: sta...@vger.kernel.org
---
include/linux/net.h | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/include/linux/net.h b/include/linux/net.h

index d48ff1180879..ae713c851342 100644

[Coly Li]

If a page sent into kernel_sendpage() is a slab page or it doesn't have
ref_count, this page is improper to send by the zero copy sendpage()
method. Otherwise such page might be unexpected released in network code
path and causes impredictable panic due to kernel memory management data
structure corruption.

This path adds a WARN_ON() on the sending page before sends it into the
concrete zero-copy sendpage() method, if the page is improper for the
zero-copy sendpage() method, a warning message can be observed before
the consequential unpredictable kernel panic.

This patch does not change existing kernel_sendpage() behavior for the
improper page zero-copy send, it just provides hint warning message for
following potential panic due the kernel memory heap corruption.

Signed-off-by: Coly Li <col...@suse.de>

Cc: Cong Wang <amw...@redhat.com>
Cc: Christoph Hellwig <h...@lst.de>

Cc: David S. Miller <da...@davemloft.net>

[Coly Li]

Currently nvme_tcp_try_send_data() doesn't use kernel_sendpage() to
send slab pages. But for pages allocated by __get_free_pages() without
__GFP_COMP, which also have refcount as 0, they are still sent by
kernel_sendpage() to remote end, this is problematic.

The new introduced helper sendpage_ok() checks both PageSlab tag and
page_count counter, and returns true if the checking page is OK to be
sent by kernel_sendpage().

This patch fixes the page checking issue of nvme_tcp_try_send_data()
with sendpage_ok(). If sendpage_ok() returns true, send this page by

kernel_sendpage(), otherwise use sock_no_sendpage to handle this page.

Signed-off-by: Coly Li <col...@suse.de>
Cc: Chaitanya Kulkarni <chaitanya...@wdc.com>
Cc: Christoph Hellwig <h...@lst.de>
Cc: Hannes Reinecke <ha...@suse.de>
Cc: Jan Kara <ja...@suse.com>
Cc: Jens Axboe <ax...@kernel.dk>

Cc: Mikhail Skorzhinskii <mskorz...@solarflare.com>
Cc: Philipp Reisner <philipp...@linbit.com>
Cc: Sagi Grimberg <sa...@grimberg.me>

Cc: Vlastimil Babka <vba...@suse.com>
Cc: sta...@vger.kernel.org
---

[Coly Li]

commit a10674bf2406 ("tcp: detecting the misuse of .sendpage for Slab
objects") adds the checks for Slab pages, but the pages don't have
page_count are still missing from the check.

Network layer's sendpage method is not designed to send page_count 0
pages neither, therefore both PageSlab() and page_count() should be

both checked for the sending page. This is exactly what sendpage_ok()
does.

This patch uses sendpage_ok() in do_tcp_sendpages() to detect misused
.sendpage, to make the code more robust.

Fixes: a10674bf2406 ("tcp: detecting the misuse of .sendpage for Slab objects")
Suggested-by: Eric Dumazet <eric.d...@gmail.com>
Signed-off-by: Coly Li <col...@suse.de>

Cc: Vasily Averin <v...@virtuozzo.com>

Cc: David S. Miller <da...@davemloft.net>

[Coly Li]

In _drbd_send_page() a page is checked by following code before sending
it by kernel_sendpage(),
(page_count(page) < 1) || PageSlab(page)
If the check is true, this page won't be send by kernel_sendpage() and
handled by sock_no_sendpage().

This kind of check is exactly what macro sendpage_ok() does, which is
introduced into include/linux/net.h to solve a similar send page issue
in nvme-tcp code.

This patch uses macro sendpage_ok() to replace the open coded checks to

page type and refcount in _drbd_send_page(), as a code cleanup.

Signed-off-by: Coly Li <col...@suse.de>

Cc: Philipp Reisner <philipp...@linbit.com>
Cc: Sagi Grimberg <sa...@grimberg.me>

---
drivers/block/drbd/drbd_main.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

[Coly Li]

In iscsci driver, iscsi_tcp_segment_map() uses the following code to
check whether the page should or not be handled by sendpage:
if (!recv && page_count(sg_page(sg)) >= 1 && !PageSlab(sg_page(sg)))

The "page_count(sg_page(sg)) >= 1 && !PageSlab(sg_page(sg)" part is to
make sure the page can be sent to network layer's zero copy path. This
part is exactly what sendpage_ok() does.

This patch uses use sendpage_ok() in iscsi_tcp_segment_map() to replace
the original open coded checks.

Signed-off-by: Coly Li <col...@suse.de>

Acked-by: Martin K. Petersen <martin....@oracle.com>

Cc: Vasily Averin <v...@virtuozzo.com>
Cc: Cong Wang <amw...@redhat.com>
Cc: Mike Christie <mich...@cs.wisc.edu>
Cc: Lee Duncan <ldu...@suse.com>

Cc: Chris Leech <cle...@redhat.com>
Cc: Christoph Hellwig <h...@lst.de>

Cc: Hannes Reinecke <ha...@suse.de>
---

drivers/scsi/libiscsi_tcp.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/libiscsi_tcp.c b/drivers/scsi/libiscsi_tcp.c
index 37e5d4e48c2f..83f14b2c8804 100644
--- a/drivers/scsi/libiscsi_tcp.c
+++ b/drivers/scsi/libiscsi_tcp.c
@@ -128,7 +128,7 @@ static void iscsi_tcp_segment_map(struct iscsi_segment *segment, int recv)

* coalescing neighboring slab objects into a single frag which
* triggers one of hardened usercopy checks.
*/

[Coly Li]

In libceph, ceph_tcp_sendpage() does the following checks before handle
the page by network layer's zero copy sendpage method,
if (page_count(page) >= 1 && !PageSlab(page))

This check is exactly what sendpage_ok() does. This patch replace the
open coded checks by sendpage_ok() as a code cleanup.

Signed-off-by: Coly Li <col...@suse.de>
Acked-by: Jeff Layton <jla...@kernel.org>
Cc: Ilya Dryomov <idry...@gmail.com>
---
net/ceph/messenger.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index bdfd66ba3843..d4d7a0e52491 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c

@@ -575,7 +575,7 @@ static int ceph_tcp_sendpage(struct socket *sock, struct page *page,

* coalescing neighboring slab objects into a single frag which
* triggers one of hardened usercopy checks.
*/

- if (page_count(page) >= 1 && !PageSlab(page))
+ if (sendpage_ok(page))
sendpage = sock->ops->sendpage;
else
sendpage = sock_no_sendpage;

--
2.26.2

[Ulrich Windl]

>>> Coly Li <col...@suse.de> schrieb am 01.10.2020 um 09:54 in Nachricht
<20201001075408...@suse.de>:

> If a page sent into kernel_sendpage() is a slab page or it doesn't have
> ref_count, this page is improper to send by the zero copy sendpage()
> method. Otherwise such page might be unexpected released in network code

s/unexpected/unexpectedly/

> path and causes impredictable panic due to kernel memory management data
> structure corruption.
>
> This path adds a WARN_ON() on the sending page before sends it into the
> concrete zero-copy sendpage() method, if the page is improper for the
> zero-copy sendpage() method, a warning message can be observed before
> the consequential unpredictable kernel panic.
>
> This patch does not change existing kernel_sendpage() behavior for the
> improper page zero-copy send, it just provides hint warning message for
> following potential panic due the kernel memory heap corruption.
>
> Signed-off-by: Coly Li <col...@suse.de>
> Cc: Cong Wang <amw...@redhat.com>
> Cc: Christoph Hellwig <h...@lst.de>
> Cc: David S. Miller <da...@davemloft.net>
> Cc: Sridhar Samudrala <s...@us.ibm.com>

...

[Lee Duncan]

Reviewed-by: Lee Duncan <ldu...@suse.com>

[Coly Li]

As Sagi Grimberg suggested, the original fix is refind to a more common
inline routine:

static inline bool sendpage_ok(struct page *page)

{
return (!PageSlab(page) && page_count(page) >= 1);
}
If sendpage_ok() returns true, the checking page can be handled by the
concrete zero-copy sendpage method in network layer.

The v10 series has 7 patches, fixes a WARN_ONCE() usage from v9 series,

- The 1st patch in this series introduces sendpage_ok() in header file
include/linux/net.h.
- The 2nd patch adds WARN_ONCE() for improper zero-copy send in
kernel_sendpage().
- The 3rd patch fixes the page checking issue in nvme-over-tcp driver.
- The 4th patch adds page_count check by using sendpage_ok() in
do_tcp_sendpages() as Eric Dumazet suggested.
- The 5th and 6th patches just replace existing open coded checks with
the inline sendpage_ok() routine.

Coly Li

Cc: Chaitanya Kulkarni <chaitanya...@wdc.com>

Cc: Chris Leech <cle...@redhat.com>
Cc: Christoph Hellwig <h...@lst.de>

Cc: Cong Wang <amw...@redhat.com>

Cc: David S. Miller <da...@davemloft.net>

Cc: Eric Dumazet <eric.d...@gmail.com>
Cc: Hannes Reinecke <ha...@suse.de>
Cc: Ilya Dryomov <idry...@gmail.com>
Cc: Jan Kara <ja...@suse.com>

Cc: Jeff Layton <jla...@kernel.org>
Cc: Jens Axboe <ax...@kernel.dk>

Cc: Lee Duncan <ldu...@suse.com>

Cc: Mike Christie <mich...@cs.wisc.edu>
Cc: Mikhail Skorzhinskii <mskorz...@solarflare.com>

Cc: Philipp Reisner <philipp...@linbit.com>
Cc: Sagi Grimberg <sa...@grimberg.me>

Cc: Vasily Averin <v...@virtuozzo.com>

Cc: Vlastimil Babka <vba...@suse.com>
---
Changelog:

v10, fix WARN_ONCE() usage, and add Reivewed-by tag from Lee Duncan.

[Coly Li]

The original problem was from nvme-over-tcp code, who mistakenly uses
kernel_sendpage() to send pages allocated by __get_free_pages() without
__GFP_COMP flag. Such pages don't have refcount (page_count is 0) on
tail pages, sending them by kernel_sendpage() may trigger a kernel panic
from a corrupted kernel heap, because these pages are incorrectly freed
in network stack as page_count 0 pages.

This patch introduces a helper sendpage_ok(), it returns true if the
checking page,
- is not slab page: PageSlab(page) is false.
- has page refcount: page_count(page) is not zero

All drivers who want to send page to remote end by kernel_sendpage()
may use this helper to check whether the page is OK. If the helper does
not return true, the driver should try other non sendpage method (e.g.

sock_no_sendpage()) to handle the page.

Signed-off-by: Coly Li <col...@suse.de>
Cc: Chaitanya Kulkarni <chaitanya...@wdc.com>
Cc: Christoph Hellwig <h...@lst.de>
Cc: Hannes Reinecke <ha...@suse.de>
Cc: Jan Kara <ja...@suse.com>
Cc: Jens Axboe <ax...@kernel.dk>

Cc: Mikhail Skorzhinskii <mskorz...@solarflare.com>
Cc: Philipp Reisner <philipp...@linbit.com>
Cc: Sagi Grimberg <sa...@grimberg.me>

Cc: Vlastimil Babka <vba...@suse.com>
Cc: sta...@vger.kernel.org
---

[Coly Li]

If a page sent into kernel_sendpage() is a slab page or it doesn't have
ref_count, this page is improper to send by the zero copy sendpage()
method. Otherwise such page might be unexpected released in network code

path and causes impredictable panic due to kernel memory management data
structure corruption.

This path adds a WARN_ON() on the sending page before sends it into the
concrete zero-copy sendpage() method, if the page is improper for the
zero-copy sendpage() method, a warning message can be observed before
the consequential unpredictable kernel panic.

This patch does not change existing kernel_sendpage() behavior for the
improper page zero-copy send, it just provides hint warning message for
following potential panic due the kernel memory heap corruption.

Signed-off-by: Coly Li <col...@suse.de>

Cc: Cong Wang <amw...@redhat.com>
Cc: Christoph Hellwig <h...@lst.de>

Cc: David S. Miller <da...@davemloft.net>

Cc: Sridhar Samudrala <s...@us.ibm.com>
---
net/socket.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/socket.c b/net/socket.c

index 0c0144604f81..58cac2da5f66 100644

--- a/net/socket.c
+++ b/net/socket.c
@@ -3638,9 +3638,11 @@ EXPORT_SYMBOL(kernel_getpeername);
int kernel_sendpage(struct socket *sock, struct page *page, int offset,
size_t size, int flags)
{
- if (sock->ops->sendpage)
+ if (sock->ops->sendpage) {
+ /* Warn in case the improper page to zero-copy send */

+ WARN_ONCE(!sendpage_ok(page), "improper page for zero-copy send");

[Coly Li]

Currently nvme_tcp_try_send_data() doesn't use kernel_sendpage() to
send slab pages. But for pages allocated by __get_free_pages() without
__GFP_COMP, which also have refcount as 0, they are still sent by
kernel_sendpage() to remote end, this is problematic.

The new introduced helper sendpage_ok() checks both PageSlab tag and
page_count counter, and returns true if the checking page is OK to be
sent by kernel_sendpage().

This patch fixes the page checking issue of nvme_tcp_try_send_data()
with sendpage_ok(). If sendpage_ok() returns true, send this page by

kernel_sendpage(), otherwise use sock_no_sendpage to handle this page.

Signed-off-by: Coly Li <col...@suse.de>
Cc: Chaitanya Kulkarni <chaitanya...@wdc.com>
Cc: Christoph Hellwig <h...@lst.de>
Cc: Hannes Reinecke <ha...@suse.de>
Cc: Jan Kara <ja...@suse.com>
Cc: Jens Axboe <ax...@kernel.dk>

Cc: Mikhail Skorzhinskii <mskorz...@solarflare.com>
Cc: Philipp Reisner <philipp...@linbit.com>
Cc: Sagi Grimberg <sa...@grimberg.me>

Cc: Vlastimil Babka <vba...@suse.com>
Cc: sta...@vger.kernel.org
---

[Coly Li]

commit a10674bf2406 ("tcp: detecting the misuse of .sendpage for Slab
objects") adds the checks for Slab pages, but the pages don't have
page_count are still missing from the check.

Network layer's sendpage method is not designed to send page_count 0
pages neither, therefore both PageSlab() and page_count() should be

both checked for the sending page. This is exactly what sendpage_ok()
does.

This patch uses sendpage_ok() in do_tcp_sendpages() to detect misused
.sendpage, to make the code more robust.

Fixes: a10674bf2406 ("tcp: detecting the misuse of .sendpage for Slab objects")
Suggested-by: Eric Dumazet <eric.d...@gmail.com>
Signed-off-by: Coly Li <col...@suse.de>

Cc: Vasily Averin <v...@virtuozzo.com>

Cc: David S. Miller <da...@davemloft.net>

[Coly Li]

In _drbd_send_page() a page is checked by following code before sending
it by kernel_sendpage(),
(page_count(page) < 1) || PageSlab(page)
If the check is true, this page won't be send by kernel_sendpage() and
handled by sock_no_sendpage().

This kind of check is exactly what macro sendpage_ok() does, which is
introduced into include/linux/net.h to solve a similar send page issue
in nvme-tcp code.

This patch uses macro sendpage_ok() to replace the open coded checks to

page type and refcount in _drbd_send_page(), as a code cleanup.

Signed-off-by: Coly Li <col...@suse.de>

Cc: Philipp Reisner <philipp...@linbit.com>
Cc: Sagi Grimberg <sa...@grimberg.me>

---
drivers/block/drbd/drbd_main.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

[Coly Li]

In iscsci driver, iscsi_tcp_segment_map() uses the following code to
check whether the page should or not be handled by sendpage:
if (!recv && page_count(sg_page(sg)) >= 1 && !PageSlab(sg_page(sg)))

The "page_count(sg_page(sg)) >= 1 && !PageSlab(sg_page(sg)" part is to
make sure the page can be sent to network layer's zero copy path. This
part is exactly what sendpage_ok() does.

This patch uses use sendpage_ok() in iscsi_tcp_segment_map() to replace
the original open coded checks.

Signed-off-by: Coly Li <col...@suse.de>

Reviewed-by: Lee Duncan <ldu...@suse.com>

Acked-by: Martin K. Petersen <martin....@oracle.com>

Cc: Vasily Averin <v...@virtuozzo.com>
Cc: Cong Wang <amw...@redhat.com>
Cc: Mike Christie <mich...@cs.wisc.edu>

Cc: Chris Leech <cle...@redhat.com>
Cc: Christoph Hellwig <h...@lst.de>

Cc: Hannes Reinecke <ha...@suse.de>
---

drivers/scsi/libiscsi_tcp.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/libiscsi_tcp.c b/drivers/scsi/libiscsi_tcp.c
index 37e5d4e48c2f..83f14b2c8804 100644
--- a/drivers/scsi/libiscsi_tcp.c
+++ b/drivers/scsi/libiscsi_tcp.c
@@ -128,7 +128,7 @@ static void iscsi_tcp_segment_map(struct iscsi_segment *segment, int recv)
* coalescing neighboring slab objects into a single frag which
* triggers one of hardened usercopy checks.
*/
- if (!recv && page_count(sg_page(sg)) >= 1 && !PageSlab(sg_page(sg)))
+ if (!recv && sendpage_ok(sg_page(sg)))
return;

if (recv) {

--
2.26.2

[Coly Li]

In libceph, ceph_tcp_sendpage() does the following checks before handle
the page by network layer's zero copy sendpage method,
if (page_count(page) >= 1 && !PageSlab(page))

This check is exactly what sendpage_ok() does. This patch replace the
open coded checks by sendpage_ok() as a code cleanup.

Signed-off-by: Coly Li <col...@suse.de>

Acked-by: Jeff Layton <jla...@kernel.org>
Cc: Ilya Dryomov <idry...@gmail.com>
---

net/ceph/messenger.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index bdfd66ba3843..d4d7a0e52491 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c

@@ -575,7 +575,7 @@ static int ceph_tcp_sendpage(struct socket *sock, struct page *page,

* coalescing neighboring slab objects into a single frag which
* triggers one of hardened usercopy checks.
*/

[Coly Li]

On 2020/10/2 03:48, David Miller wrote:
> From: David Miller <da...@davemloft.net>
> Date: Thu, 01 Oct 2020 12:43:45 -0700 (PDT)
>
>> Series applied and queued up for -stable, thank you.
>
> Actually, this doesn't even build:
>
> In file included from ./arch/x86/include/asm/bug.h:93,
> from ./include/linux/bug.h:5,
> from ./include/linux/mmdebug.h:5,
> from ./include/linux/mm.h:9,
> from net/socket.c:55:
> net/socket.c: In function ‘kernel_sendpage’:
> ./include/asm-generic/bug.h:97:3: error: too few arguments to function ‘__warn_printk’
> 97 | __warn_printk(arg); \
> | ^~~~~~~~~~~~~
>
> Was this even build tested?
>

Hi David,

Obviously my fault and no excuse for leaking this uncompleted version to
you. I just re-post a v10 version which I make sure all patches are the
latest version.

Sorry for the inconvenience and thank you in advance for taking this set.

Coly Li

[David Miller]

[David Miller]

From: Coly Li <col...@suse.de>
Date: Fri, 2 Oct 2020 16:27:27 +0800

> As Sagi Grimberg suggested, the original fix is refind to a more common
> inline routine:
> static inline bool sendpage_ok(struct page *page)
> {
> return (!PageSlab(page) && page_count(page) >= 1);
> }
> If sendpage_ok() returns true, the checking page can be handled by the
> concrete zero-copy sendpage method in network layer.

Series applied.

> The v10 series has 7 patches, fixes a WARN_ONCE() usage from v9 series,

...

I still haven't heard from you how such a fundamental build failure
was even possible.

If the v9 patch series did not even compile, how in the world did you
perform functional testing of these changes?

Please explain this to me, instead of just quietly fixing it and
posting an updated series.

Thank you.

[David Miller]

From: Coly Li <col...@suse.de>
Date: Fri, 2 Oct 2020 16:30:12 +0800

> Obviously my fault and no excuse for leaking this uncompleted version to
> you. I just re-post a v10 version which I make sure all patches are the
> latest version.
>
> Sorry for the inconvenience and thank you in advance for taking this set.

How did this happen?

How did you functionally test the patch set if it didn't even compile?

I want you to explain why you sent a completely untested patch set.

[David Miller]

From: Coly Li <col...@suse.de>
Date: Thu, 1 Oct 2020 15:54:01 +0800

> This series was original by a bug fix in nvme-over-tcp driver which only
> checked whether a page was allocated from slab allcoator, but forgot to
> check its page_count: The page handled by sendpage should be neither a
> Slab page nor 0 page_count page.
>
> As Sagi Grimberg suggested, the original fix is refind to a more common
> inline routine:
> static inline bool sendpage_ok(struct page *page)
> {
> return (!PageSlab(page) && page_count(page) >= 1);
> }
> If sendpage_ok() returns true, the checking page can be handled by the
> concrete zero-copy sendpage method in network layer.
>
> The v9 series has 7 patches, no change from v8 series,
> - The 1st patch in this series introduces sendpage_ok() in header file
> include/linux/net.h.
> - The 2nd patch adds WARN_ONCE() for improper zero-copy send in
> kernel_sendpage().
> - The 3rd patch fixes the page checking issue in nvme-over-tcp driver.
> - The 4th patch adds page_count check by using sendpage_ok() in
> do_tcp_sendpages() as Eric Dumazet suggested.
> - The 5th and 6th patches just replace existing open coded checks with

...

[Coly Li]

On 2020/10/3 06:28, David Miller wrote:
> From: Coly Li <col...@suse.de>
> Date: Fri, 2 Oct 2020 16:27:27 +0800
>
>> As Sagi Grimberg suggested, the original fix is refind to a more common
>> inline routine:
>> static inline bool sendpage_ok(struct page *page)
>> {
>> return (!PageSlab(page) && page_count(page) >= 1);
>> }
>> If sendpage_ok() returns true, the checking page can be handled by the
>> concrete zero-copy sendpage method in network layer.
>
> Series applied.
>
>> The v10 series has 7 patches, fixes a WARN_ONCE() usage from v9 series,
> ...
>
> I still haven't heard from you how such a fundamental build failure
> was even possible.
>

Hi David,

Here is the detail steps how I leaked this uncompleted patch to you,
1) Add WARN_ONCE() as WARN_ON() to kernel_sendpage(). Maybe I was still
hesitating when I typed WARN_ONCE() on keyboard.
2) Generate the patches, prepare to post
3) Hmm, compiling failed, oh it is WARN_ONCE(). Yeah, WARN_ONCE() might
be more informative and better.
4) Modify to use WARN_ONCE() and compile and try, looks fine.
5) Re-generate the patches to overwrite the previous ones.
6) Post the patches.

The missing part was, before I post the patches, I should do rebase and
commit the change, but (interrupted by other stuffs) it skipped in my
mind. Although I regenerated the series but the change was not included.
The result was, uncompleted patch posted and the second-half change
still stayed in my local file.

> If the v9 patch series did not even compile, how in the world did you
> perform functional testing of these changes?
>

Only 0002-net-add-WARN_ONCE-in-kernel_sendpage-for-improper-ze.patch was
tested in v9 series, other tests were done in previous versions.

> Please explain this to me, instead of just quietly fixing it and
> posting an updated series.

And not all the patches in the series were tested. Here is the testing
coverage of the series:

The following ones were tested and verified to break nothing and avoid
the mm corruption and panic,
0001-net-introduce-helper-sendpage_ok-in-include-linux-ne.patch
0002-net-add-WARN_ONCE-in-kernel_sendpage-for-improper-ze.patch
0003-nvme-tcp-check-page-by-sendpage_ok-before-calling-ke.patch
0006-scsi-libiscsi-use-sendpage_ok-in-iscsi_tcp_segment_m.patch

The following ones were not tested, due to complicated environment setup,
0005-drbd-code-cleanup-by-using-sendpage_ok-to-check-page.patch
0007-libceph-use-sendpage_ok-in-ceph_tcp_sendpage.patch

This patch I didn't explicitly test, due to lack of knowledge to modify
network code to trigger a buggy condition. It just went with other
tested patches,
0004-tcp-use-sendpage_ok-to-detect-misused-.sendpage.patch


Back to the built failure, I don't have excuse for leaking this
uncompleted version to you. Of cause I will try to avoid to
inefficiently occupy maintainer's time by such silly mess up.

Thanks for your review and the thorough maintenance.

Coly Li

[Coly Li]

On 2020/10/3 06:28, David Miller wrote:

> From: Coly Li <col...@suse.de>
> Date: Fri, 2 Oct 2020 16:27:27 +0800
>
>> As Sagi Grimberg suggested, the original fix is refind to a more common
>> inline routine:
>> static inline bool sendpage_ok(struct page *page)
>> {
>> return (!PageSlab(page) && page_count(page) >= 1);
>> }
>> If sendpage_ok() returns true, the checking page can be handled by the
>> concrete zero-copy sendpage method in network layer.
>
> Series applied.
>
>> The v10 series has 7 patches, fixes a WARN_ONCE() usage from v9 series,
> ...
>
> I still haven't heard from you how such a fundamental build failure
> was even possible.
>

Hi David,

Here is the detail steps how I leaked this uncompleted patch to you,
1) Add WARN_ONCE() as WARN_ON() to kernel_sendpage(). Maybe I was still
hesitating when I typed WARN_ONCE() on keyboard.
2) Generate the patches, prepare to post
3) Hmm, compiling failed, oh it is WARN_ONCE(). Yeah, WARN_ONCE() might
be more informative and better.
4) Modify to use WARN_ONCE() and compile and try, looks fine.
5) Re-generate the patches to overwrite the previous ones.
6) Post the patches.

The missing part was, before I post the patches, I should do rebase and
commit the change, but (interrupted by other stuffs) it skipped in my
mind. Although I regenerated the series but the change was not included.
The result was, uncompleted patch posted and the second-half change
still stayed in my local file.

> If the v9 patch series did not even compile, how in the world did you
> perform functional testing of these changes?
>

Only 0002-net-add-WARN_ONCE-in-kernel_sendpage-for-improper-ze.patch was
tested in v9 series, other tests were done in previous versions.

> Please explain this to me, instead of just quietly fixing it and
> posting an updated series.