[ndctl PATCH] test/fwctl: Add Get Feature OOB rejection regression test

[ndctl PATCH] test/fwctl: Add Get Feature OOB rejection regression test

[Richard Cheng]

Add a negative case to the CXL fwctl test that issues a Get Feature
FWCTL_RPC with out_len == offset(struct fwctl_rpc_cxl_out, payload) and
a non-zero count. The kernel must reject this with -EINVAL instead of
writing the feature payload past the rpc_out buffer.

This is the userspace regression test for corresponding kernel fix [1].

[1]: https://lore.kernel.org/all/20260624134737.49166-1-icheng@nvidia.com/
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
 test/fwctl.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/test/fwctl.c b/test/fwctl.c
index 979c1a6..69d0048 100644
--- a/test/fwctl.c
+++ b/test/fwctl.c
@@ -5,6 +5,7 @@
 #include <stdio.h>
 #include <endian.h>
 #include <stdint.h>
+#include <stddef.h>
 #include <stdlib.h>
 #include <syslog.h>
 #include <string.h>
@@ -207,6 +208,45 @@ out:
 	return rc;
 }
 
+static int cxl_fwctl_rpc_get_feature_oob(int fd, struct test_feature *feat_ctx)
+{
+	struct cxl_mbox_get_feat_in *feat_in;
+	struct fwctl_rpc_cxl_out *out;
+	size_t out_size, in_size;
+	struct fwctl_rpc_cxl *in;
+	struct fwctl_rpc *rpc;
+	int rc;
+
+	in_size = sizeof(*in) + sizeof(*feat_in);
+	/* header only => zero payload room */
+	out_size = offsetof(struct fwctl_rpc_cxl_out, payload);
+
+	rpc = get_prepped_command(in_size, out_size,
+				  CXL_MBOX_OPCODE_GET_FEATURE);
+	if (!rpc)
+		return -ENXIO;
+
+	in = (struct fwctl_rpc_cxl *)rpc->in;
+	out = (struct fwctl_rpc_cxl_out *)rpc->out;
+
+	feat_in = &in->get_feat_in;
+	uuid_copy(feat_in->uuid, feat_ctx->uuid);
+	/* non-zero count that exceeds the zero payload room */
+	feat_in->count = feat_ctx->get_size;
+
+	rc = send_command(fd, rpc, out);
+	free_rpc(rpc);
+
+	if (rc == -EINVAL)
+		return 0;
+	if (rc == 0) {
+		fprintf(stderr, "Get Feature with undersized out_len was not rejected\n");
+		return -ENXIO;
+	}
+	fprintf(stderr, "Get Feature OOB rejection test: unexpected rc %d\n", rc);
+	return rc;
+}
+
 static int cxl_fwctl_rpc_set_test_feature(int fd, struct test_feature *feat_ctx)
 {
 	struct cxl_mbox_set_feat_in *feat_in;
@@ -393,6 +433,12 @@ static int test_fwctl_features(struct cxl_memdev *memdev)
 		goto out;
 	}
 
+	rc = cxl_fwctl_rpc_get_feature_oob(fd, &feat_ctx);
+	if (rc) {
+		fprintf(stderr, "Failed Get Feature OOB rejection test: %d\n", rc);
+		goto out;
+	}
+
 out:
 	close(fd);
 	return rc;

base-commit: 8ad90e54f0ff4f7291e7f21d44d769d10f24e2b6
-- 
2.43.0

Re: [ndctl PATCH] test/fwctl: Add Get Feature OOB rejection regression test

[Alison Schofield]

On Wed, Jun 24, 2026 at 10:00:06PM +0800, Richard Cheng wrote:
> Add a negative case to the CXL fwctl test that issues a Get Feature
> FWCTL_RPC with out_len == offset(struct fwctl_rpc_cxl_out, payload) and
> a non-zero count. The kernel must reject this with -EINVAL instead of
> writing the feature payload past the rpc_out buffer.
> 
> This is the userspace regression test for corresponding kernel fix [1].
> 
> [1]: https://lore.kernel.org/all/20260624134737.49166-1-icheng@nvidia.com/
> Signed-off-by: Richard Cheng <icheng@nvidia.com>

Thanks Richard! 

In the unit tests when we have cases that depend on a specific 
kernel fix landing we prefer to gate on that kver. This is a
first though, because the gate needs to be within the C program
not simply using the check_min_kver helper as is done for the
test shell scripts.

I think something like appended would be useful here. See if that
works for you.

I only tested without the fix to confirm it fails the entire fwctl
test. I also stopped short of testing with the fix because I see
another patchset in flight grouping bounds checks and figure you
will come back around and update this test patch similarly.


diff --git a/test/fwctl.c b/test/fwctl.c
index 69d0048c09df..b18a4f10717b 100644
--- a/test/fwctl.c
+++ b/test/fwctl.c
@@ -6,10 +6,12 @@
 #include <endian.h>
 #include <stdint.h>
 #include <stddef.h>
+#include <stdbool.h>
 #include <stdlib.h>
 #include <syslog.h>
 #include <string.h>
 #include <unistd.h>
+#include <sys/utsname.h>
 #include <sys/ioctl.h>
 #include <cxl/libcxl.h>
 #include <linux/uuid.h>
@@ -21,6 +23,37 @@

 static const char provider[] = "cxl_test";

+/* Running kernel version parsed once in main(). */
+static unsigned int kver_major;
+static unsigned int kver_minor;
+
+/*
+ * kver_ge - is the running kernel at least major.minor?
+ *
+ * The C version of the shell suite's check_min_kver helper.
+ * Test cases for fixes tied to a specific kver, gate here so that test
+ * cases quietly skip rather than fail on kernels that predate the fix.
+ * Acknowledging that doesn't help testing of backports.
+ */
+static bool kver_ge(unsigned int major, unsigned int minor)
+{
+       if (kver_major != major)
+               return kver_major > major;
+       return kver_minor >= minor;
+}
+
+static void parse_kver(void)
+{
+       struct utsname uts;
+
+       if (uname(&uts) == 0 &&
+           sscanf(uts.release, "%u.%u", &kver_major, &kver_minor) == 2)
+               return;
+
+       kver_major = 0;
+       kver_minor = 0;
+}
+
 UUID_DEFINE(test_uuid,
            0xff, 0xff, 0xff, 0xff,
            0xff, 0xff,
@@ -208,6 +241,10 @@ out:
        return rc;
 }

+/* First kernel release with the Get Feature OOB rejection fix */
+#define GET_FEAT_OOB_FIX_MAJOR 7
+#define GET_FEAT_OOB_FIX_MINOR 3
+
 static int cxl_fwctl_rpc_get_feature_oob(int fd, struct test_feature *feat_ctx)
 {
        struct cxl_mbox_get_feat_in *feat_in;
@@ -217,6 +254,13 @@ static int cxl_fwctl_rpc_get_feature_oob(int fd, struct test_feature *feat_ctx)
        struct fwctl_rpc *rpc;
        int rc;

+       if (!kver_ge(GET_FEAT_OOB_FIX_MAJOR, GET_FEAT_OOB_FIX_MINOR)) {
+               fprintf(stderr,
+                       "skip: Get Feature OOB rejection test needs kernel >= %u.%u\n",
+                       GET_FEAT_OOB_FIX_MAJOR, GET_FEAT_OOB_FIX_MINOR);
+               return 0;
+       }
+
        in_size = sizeof(*in) + sizeof(*feat_in);
        /* header only => zero payload room */
        out_size = offsetof(struct fwctl_rpc_cxl_out, payload);
@@ -463,6 +507,8 @@ int main(int argc, char *argv[])
        struct cxl_bus *bus;
        int rc;

+       parse_kver();
+
        rc = cxl_new(&ctx);
        if (rc < 0)
                return rc;

Re: [ndctl PATCH] test/fwctl: Add Get Feature OOB rejection regression test

[Alison Schofield]

On Wed, Jun 24, 2026 at 10:00:06PM +0800, Richard Cheng wrote:
> Add a negative case to the CXL fwctl test that issues a Get Feature
> FWCTL_RPC with out_len == offset(struct fwctl_rpc_cxl_out, payload) and
> a non-zero count. The kernel must reject this with -EINVAL instead of
> writing the feature payload past the rpc_out buffer.
> 
> This is the userspace regression test for corresponding kernel fix [1].

Hi Richard,

I just finished reviewing the now 3 piece series[2], that [1] is now
a piece of.

One suggestion on top of the kver gating suggested in prior response
is to add a companion negative case for the Set Feature bounds fix
in the same series. Same shape as this one, ie build a normal Set
Feature RPC, set out_len to 0, expect -EINVAL. It's a stronger backstop
than the Get case, too because before the fix an out_len of 0 makes
kvzalloc() return ZERO_SIZE_PTR, which passes the !rpc_out check, and
the header write then oopses rather than just returning a wrong status.
Gate it on the same kver helper.

I'm stopping short of suggesting a test for the third patch (the Get
Feature per-iteration clamp). That one looks like it needs a
multi-transfer feature and a device that over returns on the last chunk,
neither possible without a cxl_test mock change.

-- Alison

> [1]: https://lore.kernel.org/all/20260624134737.49166-1-icheng@nvidia.com/
[2]: https://lore.kernel.org/linux-cxl/20260626104102.53892-1-icheng@nvidia.com/#r


> Signed-off-by: Richard Cheng <icheng@nvidia.com>
> ---
>  test/fwctl.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/test/fwctl.c b/test/fwctl.c
> index 979c1a6..69d0048 100644
> --- a/test/fwctl.c
> +++ b/test/fwctl.c
> @@ -5,6 +5,7 @@
>  #include <stdio.h>
>  #include <endian.h>
>  #include <stdint.h>
> +#include <stddef.h>
>  #include <stdlib.h>
>  #include <syslog.h>
>  #include <string.h>
> @@ -207,6 +208,45 @@ out:
>  	return rc;
>  }
>  
> +static int cxl_fwctl_rpc_get_feature_oob(int fd, struct test_feature *feat_ctx)
> +{
> +	struct cxl_mbox_get_feat_in *feat_in;
> +	struct fwctl_rpc_cxl_out *out;
> +	size_t out_size, in_size;
> +	struct fwctl_rpc_cxl *in;
> +	struct fwctl_rpc *rpc;
> +	int rc;
> +
> +	in_size = sizeof(*in) + sizeof(*feat_in);
> +	/* header only => zero payload room */
> +	out_size = offsetof(struct fwctl_rpc_cxl_out, payload);
> +
> +	rpc = get_prepped_command(in_size, out_size,
> +				  CXL_MBOX_OPCODE_GET_FEATURE);
> +	if (!rpc)
> +		return -ENXIO;
> +
> +	in = (struct fwctl_rpc_cxl *)rpc->in;
> +	out = (struct fwctl_rpc_cxl_out *)rpc->out;
> +
> +	feat_in = &in->get_feat_in;
> +	uuid_copy(feat_in->uuid, feat_ctx->uuid);
> +	/* non-zero count that exceeds the zero payload room */
> +	feat_in->count = feat_ctx->get_size;
> +
> +	rc = send_command(fd, rpc, out);
> +	free_rpc(rpc);
> +
> +	if (rc == -EINVAL)
> +		return 0;
> +	if (rc == 0) {
> +		fprintf(stderr, "Get Feature with undersized out_len was not rejected\n");
> +		return -ENXIO;
> +	}
> +	fprintf(stderr, "Get Feature OOB rejection test: unexpected rc %d\n", rc);
> +	return rc;
> +}
> +
>  static int cxl_fwctl_rpc_set_test_feature(int fd, struct test_feature *feat_ctx)
>  {
>  	struct cxl_mbox_set_feat_in *feat_in;
> @@ -393,6 +433,12 @@ static int test_fwctl_features(struct cxl_memdev *memdev)
>  		goto out;
>  	}
>  
> +	rc = cxl_fwctl_rpc_get_feature_oob(fd, &feat_ctx);
> +	if (rc) {
> +		fprintf(stderr, "Failed Get Feature OOB rejection test: %d\n", rc);
> +		goto out;
> +	}
> +
>  out:
>  	close(fd);
>  	return rc;
> 
> base-commit: 8ad90e54f0ff4f7291e7f21d44d769d10f24e2b6
> -- 
> 2.43.0
> 

Re: [ndctl PATCH] test/fwctl: Add Get Feature OOB rejection regression test

[Richard Cheng]

On Tue, Jul 14, 2026 at 12:28:28PM +0800, Alison Schofield wrote:
> On Wed, Jun 24, 2026 at 10:00:06PM +0800, Richard Cheng wrote:
> > Add a negative case to the CXL fwctl test that issues a Get Feature
> > FWCTL_RPC with out_len == offset(struct fwctl_rpc_cxl_out, payload) and
> > a non-zero count. The kernel must reject this with -EINVAL instead of
> > writing the feature payload past the rpc_out buffer.
> > 
> > This is the userspace regression test for corresponding kernel fix [1].
> 
> Hi Richard,
> 
> I just finished reviewing the now 3 piece series[2], that [1] is now
> a piece of.
> 
> One suggestion on top of the kver gating suggested in prior response
> is to add a companion negative case for the Set Feature bounds fix
> in the same series. Same shape as this one, ie build a normal Set
> Feature RPC, set out_len to 0, expect -EINVAL. It's a stronger backstop
> than the Get case, too because before the fix an out_len of 0 makes
> kvzalloc() return ZERO_SIZE_PTR, which passes the !rpc_out check, and
> the header write then oopses rather than just returning a wrong status.
> Gate it on the same kver helper.
> 
> I'm stopping short of suggesting a test for the third patch (the Get
> Feature per-iteration clamp). That one looks like it needs a
> multi-transfer feature and a device that over returns on the last chunk,
> neither possible without a cxl_test mock change.
> 
> -- Alison
> 
> > [1]: https://lore.kernel.org/all/20260624134737.49166-1-icheng@nvidia.com/
> [2]: https://lore.kernel.org/linux-cxl/20260626104102.53892-1-icheng@nvidia.com/#r
> 
>

Hi Alison,

Thanks for the review. I'll update the ndctl patch to add kernel-version check
and a Set Feature negative test with "out_len=0"

I'll use the same version check for both tsets and send a new version for it.

As for the Get Feature per-iteration clamp, I'll also work on extending cxl_test
with the required mock behavior. I'll send the cxl_test change and its regression
test as follow-up patches.

--Richard
 
> > Signed-off-by: Richard Cheng <icheng@nvidia.com>
> > ---
> >  test/fwctl.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 46 insertions(+)
> > 
> > diff --git a/test/fwctl.c b/test/fwctl.c
> > index 979c1a6..69d0048 100644
> > --- a/test/fwctl.c
> > +++ b/test/fwctl.c
> > @@ -5,6 +5,7 @@
> >  #include <stdio.h>
> >  #include <endian.h>
> >  #include <stdint.h>
> > +#include <stddef.h>
> >  #include <stdlib.h>
> >  #include <syslog.h>
> >  #include <string.h>
> > @@ -207,6 +208,45 @@ out:
> >  	return rc;
> >  }
> >  
> > +static int cxl_fwctl_rpc_get_feature_oob(int fd, struct test_feature *feat_ctx)
> > +{
> > +	struct cxl_mbox_get_feat_in *feat_in;
> > +	struct fwctl_rpc_cxl_out *out;
> > +	size_t out_size, in_size;
> > +	struct fwctl_rpc_cxl *in;
> > +	struct fwctl_rpc *rpc;
> > +	int rc;
> > +
> > +	in_size = sizeof(*in) + sizeof(*feat_in);
> > +	/* header only => zero payload room */
> > +	out_size = offsetof(struct fwctl_rpc_cxl_out, payload);
> > +
> > +	rpc = get_prepped_command(in_size, out_size,
> > +				  CXL_MBOX_OPCODE_GET_FEATURE);
> > +	if (!rpc)
> > +		return -ENXIO;
> > +
> > +	in = (struct fwctl_rpc_cxl *)rpc->in;
> > +	out = (struct fwctl_rpc_cxl_out *)rpc->out;
> > +
> > +	feat_in = &in->get_feat_in;
> > +	uuid_copy(feat_in->uuid, feat_ctx->uuid);
> > +	/* non-zero count that exceeds the zero payload room */
> > +	feat_in->count = feat_ctx->get_size;
> > +
> > +	rc = send_command(fd, rpc, out);
> > +	free_rpc(rpc);
> > +
> > +	if (rc == -EINVAL)
> > +		return 0;
> > +	if (rc == 0) {
> > +		fprintf(stderr, "Get Feature with undersized out_len was not rejected\n");
> > +		return -ENXIO;
> > +	}
> > +	fprintf(stderr, "Get Feature OOB rejection test: unexpected rc %d\n", rc);
> > +	return rc;
> > +}
> > +
> >  static int cxl_fwctl_rpc_set_test_feature(int fd, struct test_feature *feat_ctx)
> >  {
> >  	struct cxl_mbox_set_feat_in *feat_in;
> > @@ -393,6 +433,12 @@ static int test_fwctl_features(struct cxl_memdev *memdev)
> >  		goto out;
> >  	}
> >  
> > +	rc = cxl_fwctl_rpc_get_feature_oob(fd, &feat_ctx);
> > +	if (rc) {
> > +		fprintf(stderr, "Failed Get Feature OOB rejection test: %d\n", rc);
> > +		goto out;
> > +	}
> > +
> >  out:
> >  	close(fd);
> >  	return rc;
> > 
> > base-commit: 8ad90e54f0ff4f7291e7f21d44d769d10f24e2b6
> > -- 
> > 2.43.0
> >