| Message ID | 20220815154044.24733-3-Jonathan.Cameron@huawei.com |
|---|---|
| State | Accepted |
| Commit | f010c75c05299ecd65adfd31a7841eea3476ce1f |
| Delegated to: | Dan Williams |
| Headers | show |
| Series | cxl: Fix oversized LSA write payload due to header | expand |
[ apologies if this is a duplicate response ] Jonathan Cameron wrote: > Writes to the device must include an offset and size as defined in > CXL 2.0 8.2.9.5.2.4 Set LSA (Opcode 4103h) > > Fixes tag is non obvious as this code has been through several > reworks and variable names + wasn't in use until the addition > of the region code. Looks like: Fixes: 60b8f17215de ("cxl/pmem: Translate NVDIMM label commands to CXL label commands") ...to me since any transfer that got within 8 bytes of the payload_size would fail. I notice that cxl_test worked around this bug simply because the mocking does not attempt to emulate the actual mailbox transfer, just the logical data transfer. I apprecitate having QEMU to back up the unit testing.
On Mon, 15 Aug 2022 14:55:42 -0700 Dan Williams <dan.j.williams@intel.com> wrote: > [ apologies if this is a duplicate response ] > > Jonathan Cameron wrote: > > Writes to the device must include an offset and size as defined in > > CXL 2.0 8.2.9.5.2.4 Set LSA (Opcode 4103h) > > > > Fixes tag is non obvious as this code has been through several > > reworks and variable names + wasn't in use until the addition > > of the region code. > > Looks like: > > Fixes: 60b8f17215de ("cxl/pmem: Translate NVDIMM label commands to CXL label commands") > > ...to me since any transfer that got within 8 bytes of the payload_size > would fail. Makes sense - go with that one. > > I notice that cxl_test worked around this bug simply because the mocking > does not attempt to emulate the actual mailbox transfer, just the > logical data transfer. I apprecitate having QEMU to back up the unit > testing. >
On Wed, 17 Aug 2022 12:29:30 +0100 Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote: > On Mon, 15 Aug 2022 14:55:42 -0700 > Dan Williams <dan.j.williams@intel.com> wrote: > > > [ apologies if this is a duplicate response ] > > > > Jonathan Cameron wrote: > > > Writes to the device must include an offset and size as defined in > > > CXL 2.0 8.2.9.5.2.4 Set LSA (Opcode 4103h) > > > > > > Fixes tag is non obvious as this code has been through several > > > reworks and variable names + wasn't in use until the addition > > > of the region code. > > > > Looks like: > > > > Fixes: 60b8f17215de ("cxl/pmem: Translate NVDIMM label commands to CXL label commands") > > > > ...to me since any transfer that got within 8 bytes of the payload_size > > would fail. > > Makes sense - go with that one. Hi Dan, I was assuming you'd pick this up as b4 will happily pick the fixes tag out of the thread and there weren't any other comments. Let me know if you want me to resend with the tag in place. Thanks, Jonathan > > > > > I notice that cxl_test worked around this bug simply because the mocking > > does not attempt to emulate the actual mailbox transfer, just the > > logical data transfer. I apprecitate having QEMU to back up the unit > > testing. > > >
Jonathan Cameron wrote: > On Wed, 17 Aug 2022 12:29:30 +0100 > Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote: > > > On Mon, 15 Aug 2022 14:55:42 -0700 > > Dan Williams <dan.j.williams@intel.com> wrote: > > > > > [ apologies if this is a duplicate response ] > > > > > > Jonathan Cameron wrote: > > > > Writes to the device must include an offset and size as defined in > > > > CXL 2.0 8.2.9.5.2.4 Set LSA (Opcode 4103h) > > > > > > > > Fixes tag is non obvious as this code has been through several > > > > reworks and variable names + wasn't in use until the addition > > > > of the region code. > > > > > > Looks like: > > > > > > Fixes: 60b8f17215de ("cxl/pmem: Translate NVDIMM label commands to CXL label commands") > > > > > > ...to me since any transfer that got within 8 bytes of the payload_size > > > would fail. > > > > Makes sense - go with that one. > > Hi Dan, > > I was assuming you'd pick this up as b4 will happily pick the fixes > tag out of the thread and there weren't any other comments. > > Let me know if you want me to resend with the tag in place. I believe you should have gotten a notice from patchwork, but I have this queued up for v6.1-rc fixes.
diff --git a/drivers/cxl/pmem.c b/drivers/cxl/pmem.c index 7dc0a2fa1a6b..115a7b79f343 100644 --- a/drivers/cxl/pmem.c +++ b/drivers/cxl/pmem.c @@ -107,7 +107,7 @@ static int cxl_pmem_get_config_size(struct cxl_dev_state *cxlds, *cmd = (struct nd_cmd_get_config_size) { .config_size = cxlds->lsa_size, - .max_xfer = cxlds->payload_size, + .max_xfer = cxlds->payload_size - sizeof(struct cxl_mbox_set_lsa), }; return 0;
Writes to the device must include an offset and size as defined in CXL 2.0 8.2.9.5.2.4 Set LSA (Opcode 4103h) Fixes tag is non obvious as this code has been through several reworks and variable names + wasn't in use until the addition of the region code. Due to a bug in QEMU CXL emulation this overrun resulted in QEMU crashing. Reported-by: Bobo WL <lmw.bobo@gmail.com> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> --- drivers/cxl/pmem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)