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

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/[email protected]/
> [2]: https://lore.kernel.org/linux-cxl/[email protected]/#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 <[email protected]>
> > ---
> >  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
> >