[v2] cxl/memdev: automate cleanup with __free()

[PATCH v2] cxl/memdev: automate cleanup with __free()

Posted by Pranav Tyagi 3 months, 2 weeks ago

Use the scope based resource management (defined in linux/cleanup.h) to
automate the lifetime control of struct cxl_mbox_transfer_fw. This
eliminates explicit kfree() calls and makes the code more robust and
maintainable in presence of early returns.

Signed-off-by: Pranav Tyagi <pranav.tyagi03@gmail.com>
---
 drivers/cxl/core/memdev.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index f88a13adf7fa..38f4449f9740 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -7,6 +7,7 @@
 #include <linux/slab.h>
 #include <linux/idr.h>
 #include <linux/pci.h>
+#include <linux/cleanup.h>
 #include <cxlmem.h>
 #include "trace.h"
 #include "core.h"
@@ -802,11 +803,10 @@ static int cxl_mem_activate_fw(struct cxl_memdev_state *mds, int slot)
 static int cxl_mem_abort_fw_xfer(struct cxl_memdev_state *mds)
 {
 	struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox;
-	struct cxl_mbox_transfer_fw *transfer;
 	struct cxl_mbox_cmd mbox_cmd;
-	int rc;
-
-	transfer = kzalloc(struct_size(transfer, data, 0), GFP_KERNEL);
+	
+	struct cxl_mbox_transfer_fw *transfer __free(kfree) =
+		kzalloc(struct_size(transfer, data, 0), GFP_KERNEL);
 	if (!transfer)
 		return -ENOMEM;
 
@@ -821,9 +821,7 @@ static int cxl_mem_abort_fw_xfer(struct cxl_memdev_state *mds)
 
 	transfer->action = CXL_FW_TRANSFER_ACTION_ABORT;
 
-	rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
-	kfree(transfer);
-	return rc;
+	return cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
 }
 
 static void cxl_fw_cleanup(struct fw_upload *fwl)
@@ -880,7 +878,7 @@ static enum fw_upload_err cxl_fw_write(struct fw_upload *fwl, const u8 *data,
 	struct cxl_dev_state *cxlds = &mds->cxlds;
 	struct cxl_mailbox *cxl_mbox = &cxlds->cxl_mbox;
 	struct cxl_memdev *cxlmd = cxlds->cxlmd;
-	struct cxl_mbox_transfer_fw *transfer;
+	struct cxl_mbox_transfer_fw *transfer __free(kfree);
 	struct cxl_mbox_cmd mbox_cmd;
 	u32 cur_size, remaining;
 	size_t size_in;
@@ -949,7 +947,7 @@ static enum fw_upload_err cxl_fw_write(struct fw_upload *fwl, const u8 *data,
 	rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
 	if (rc < 0) {
 		rc = FW_UPLOAD_ERR_RW_ERROR;
-		goto out_free;
+		return rc;
 	}
 
 	*written = cur_size;
@@ -963,14 +961,11 @@ static enum fw_upload_err cxl_fw_write(struct fw_upload *fwl, const u8 *data,
 			dev_err(&cxlmd->dev, "Error activating firmware: %d\n",
 				rc);
 			rc = FW_UPLOAD_ERR_HW_ERROR;
-			goto out_free;
+			return rc;
 		}
 	}
 
 	rc = FW_UPLOAD_ERR_NONE;
-
-out_free:
-	kfree(transfer);
 	return rc;
 }
 
-- 
2.49.0

Re: [PATCH v2] cxl/memdev: automate cleanup with __free()

Posted by Dan Carpenter 3 months, 1 week ago

Hi Pranav,

kernel test robot noticed the following build warnings:

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Pranav-Tyagi/cxl-memdev-automate-cleanup-with-__free/20250623-164014
base:   https://git.kernel.org/pub/scm/linux/kernel/git/cxl/cxl.git next
patch link:    https://lore.kernel.org/r/20250623083841.364002-1-pranav.tyagi03%40gmail.com
patch subject: [PATCH v2] cxl/memdev: automate cleanup with __free()
config: x86_64-randconfig-161-20250627 (https://download.01.org/0day-ci/archive/20250628/202506280653.WmzTbwEN-lkp@intel.com/config)
compiler: clang version 20.1.7 (https://github.com/llvm/llvm-project 6146a88f60492b520a36f8f8f3231e15f3cc6082)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202506280653.WmzTbwEN-lkp@intel.com/

smatch warnings:
drivers/cxl/core/memdev.c:881 cxl_fw_write() error: uninitialized symbol 'transfer'.
drivers/cxl/core/memdev.c:881 cxl_fw_write() error: uninitialized symbol 'transfer'.

vim +/transfer +881 drivers/cxl/core/memdev.c

9521875bbe0055 Vishal Verma 2023-06-14  874  static enum fw_upload_err cxl_fw_write(struct fw_upload *fwl, const u8 *data,
9521875bbe0055 Vishal Verma 2023-06-14  875  				       u32 offset, u32 size, u32 *written)
9521875bbe0055 Vishal Verma 2023-06-14  876  {
aeaefabc59ec3c Dan Williams 2023-06-25  877  	struct cxl_memdev_state *mds = fwl->dd_handle;
aeaefabc59ec3c Dan Williams 2023-06-25  878  	struct cxl_dev_state *cxlds = &mds->cxlds;
8d8081cecfb994 Dave Jiang   2024-09-05  879  	struct cxl_mailbox *cxl_mbox = &cxlds->cxl_mbox;
9521875bbe0055 Vishal Verma 2023-06-14  880  	struct cxl_memdev *cxlmd = cxlds->cxlmd;
f76a702ca63e85 Pranav Tyagi 2025-06-23 @881  	struct cxl_mbox_transfer_fw *transfer __free(kfree);

cleanup magic variables need to be initialized to NULL etc.

9521875bbe0055 Vishal Verma 2023-06-14  882  	struct cxl_mbox_cmd mbox_cmd;
9521875bbe0055 Vishal Verma 2023-06-14  883  	u32 cur_size, remaining;
9521875bbe0055 Vishal Verma 2023-06-14  884  	size_t size_in;
9521875bbe0055 Vishal Verma 2023-06-14  885  	int rc;
9521875bbe0055 Vishal Verma 2023-06-14  886  
9521875bbe0055 Vishal Verma 2023-06-14  887  	*written = 0;
9521875bbe0055 Vishal Verma 2023-06-14  888  
9521875bbe0055 Vishal Verma 2023-06-14  889  	/* Offset has to be aligned to 128B (CXL-3.0 8.2.9.3.2 Table 8-57) */
9521875bbe0055 Vishal Verma 2023-06-14  890  	if (!IS_ALIGNED(offset, CXL_FW_TRANSFER_ALIGNMENT)) {
9521875bbe0055 Vishal Verma 2023-06-14  891  		dev_err(&cxlmd->dev,
9521875bbe0055 Vishal Verma 2023-06-14  892  			"misaligned offset for FW transfer slice (%u)\n",
9521875bbe0055 Vishal Verma 2023-06-14  893  			offset);
9521875bbe0055 Vishal Verma 2023-06-14  894  		return FW_UPLOAD_ERR_RW_ERROR;

This return will try to free an uninitialized pointer.

9521875bbe0055 Vishal Verma 2023-06-14  895  	}
9521875bbe0055 Vishal Verma 2023-06-14  896  
9521875bbe0055 Vishal Verma 2023-06-14  897  	/*
aeaefabc59ec3c Dan Williams 2023-06-25  898  	 * Pick transfer size based on mds->payload_size @size must bw 128-byte
aeaefabc59ec3c Dan Williams 2023-06-25  899  	 * aligned, ->payload_size is a power of 2 starting at 256 bytes, and
aeaefabc59ec3c Dan Williams 2023-06-25  900  	 * sizeof(*transfer) is 128.  These constraints imply that @cur_size
aeaefabc59ec3c Dan Williams 2023-06-25  901  	 * will always be 128b aligned.
9521875bbe0055 Vishal Verma 2023-06-14  902  	 */
8d8081cecfb994 Dave Jiang   2024-09-05  903  	cur_size = min_t(size_t, size, cxl_mbox->payload_size - sizeof(*transfer));
9521875bbe0055 Vishal Verma 2023-06-14  904  
9521875bbe0055 Vishal Verma 2023-06-14  905  	remaining = size - cur_size;
9521875bbe0055 Vishal Verma 2023-06-14  906  	size_in = struct_size(transfer, data, cur_size);
9521875bbe0055 Vishal Verma 2023-06-14  907  
aeaefabc59ec3c Dan Williams 2023-06-25  908  	if (test_and_clear_bit(CXL_FW_CANCEL, mds->fw.state))
9521875bbe0055 Vishal Verma 2023-06-14  909  		return cxl_fw_do_cancel(fwl);
9521875bbe0055 Vishal Verma 2023-06-14  910  
9521875bbe0055 Vishal Verma 2023-06-14  911  	/*
9521875bbe0055 Vishal Verma 2023-06-14  912  	 * Slot numbers are 1-indexed
9521875bbe0055 Vishal Verma 2023-06-14  913  	 * cur_slot is the 0-indexed next_slot (i.e. 'cur_slot - 1 + 1')
9521875bbe0055 Vishal Verma 2023-06-14  914  	 * Check for rollover using modulo, and 1-index it by adding 1
9521875bbe0055 Vishal Verma 2023-06-14  915  	 */
aeaefabc59ec3c Dan Williams 2023-06-25  916  	mds->fw.next_slot = (mds->fw.cur_slot % mds->fw.num_slots) + 1;
9521875bbe0055 Vishal Verma 2023-06-14  917  
9521875bbe0055 Vishal Verma 2023-06-14  918  	/* Do the transfer via mailbox cmd */
9521875bbe0055 Vishal Verma 2023-06-14  919  	transfer = kzalloc(size_in, GFP_KERNEL);
9521875bbe0055 Vishal Verma 2023-06-14  920  	if (!transfer)
9521875bbe0055 Vishal Verma 2023-06-14  921  		return FW_UPLOAD_ERR_RW_ERROR;
9521875bbe0055 Vishal Verma 2023-06-14  922  
9521875bbe0055 Vishal Verma 2023-06-14  923  	transfer->offset = cpu_to_le32(offset / CXL_FW_TRANSFER_ALIGNMENT);
9521875bbe0055 Vishal Verma 2023-06-14  924  	memcpy(transfer->data, data + offset, cur_size);

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2] cxl/memdev: automate cleanup with __free()

Posted by Robert Richter 3 months, 2 weeks ago

On 23.06.25 14:08:41, Pranav Tyagi wrote:
> Use the scope based resource management (defined in linux/cleanup.h) to
> automate the lifetime control of struct cxl_mbox_transfer_fw. This
> eliminates explicit kfree() calls and makes the code more robust and
> maintainable in presence of early returns.
> 
> Signed-off-by: Pranav Tyagi <pranav.tyagi03@gmail.com>
> ---
>  drivers/cxl/core/memdev.c | 21 ++++++++-------------
>  1 file changed, 8 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index f88a13adf7fa..38f4449f9740 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -7,6 +7,7 @@
>  #include <linux/slab.h>
>  #include <linux/idr.h>
>  #include <linux/pci.h>
> +#include <linux/cleanup.h>
>  #include <cxlmem.h>
>  #include "trace.h"
>  #include "core.h"
> @@ -802,11 +803,10 @@ static int cxl_mem_activate_fw(struct cxl_memdev_state *mds, int slot)
>  static int cxl_mem_abort_fw_xfer(struct cxl_memdev_state *mds)
>  {
>  	struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox;
> -	struct cxl_mbox_transfer_fw *transfer;
>  	struct cxl_mbox_cmd mbox_cmd;
> -	int rc;
> -
> -	transfer = kzalloc(struct_size(transfer, data, 0), GFP_KERNEL);
> +	
> +	struct cxl_mbox_transfer_fw *transfer __free(kfree) =
> +		kzalloc(struct_size(transfer, data, 0), GFP_KERNEL);

I don't see a reason for __free() here as there are no early exits.

>  	if (!transfer)
>  		return -ENOMEM;
>  
> @@ -821,9 +821,7 @@ static int cxl_mem_abort_fw_xfer(struct cxl_memdev_state *mds)
>  
>  	transfer->action = CXL_FW_TRANSFER_ACTION_ABORT;
>  
> -	rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
> -	kfree(transfer);
> -	return rc;
> +	return cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
>  }
>  
>  static void cxl_fw_cleanup(struct fw_upload *fwl)
> @@ -880,7 +878,7 @@ static enum fw_upload_err cxl_fw_write(struct fw_upload *fwl, const u8 *data,
>  	struct cxl_dev_state *cxlds = &mds->cxlds;
>  	struct cxl_mailbox *cxl_mbox = &cxlds->cxl_mbox;
>  	struct cxl_memdev *cxlmd = cxlds->cxlmd;
> -	struct cxl_mbox_transfer_fw *transfer;
> +	struct cxl_mbox_transfer_fw *transfer __free(kfree);

Jonathan already catched this.

>  	struct cxl_mbox_cmd mbox_cmd;
>  	u32 cur_size, remaining;
>  	size_t size_in;
> @@ -949,7 +947,7 @@ static enum fw_upload_err cxl_fw_write(struct fw_upload *fwl, const u8 *data,
>  	rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
>  	if (rc < 0) {
>  		rc = FW_UPLOAD_ERR_RW_ERROR;
> -		goto out_free;
> +		return rc;

If you want to remove the goto here, just free transfer right after
calling cxl_internal_send_cmd(). It is no longer used then.

I only want those cleanup helpers where they are actually useful and
do not just add complexity.

Thanks,

-Robert

>  	}
>  
>  	*written = cur_size;
> @@ -963,14 +961,11 @@ static enum fw_upload_err cxl_fw_write(struct fw_upload *fwl, const u8 *data,
>  			dev_err(&cxlmd->dev, "Error activating firmware: %d\n",
>  				rc);
>  			rc = FW_UPLOAD_ERR_HW_ERROR;
> -			goto out_free;
> +			return rc;
>  		}
>  	}
>  
>  	rc = FW_UPLOAD_ERR_NONE;
> -
> -out_free:
> -	kfree(transfer);
>  	return rc;
>  }
>  
> -- 
> 2.49.0
>

Re: [PATCH v2] cxl/memdev: automate cleanup with __free()

Posted by Pranav Tyagi 3 months, 2 weeks ago

On Tue, Jun 24, 2025 at 8:47 PM Robert Richter <rrichter@amd.com> wrote:
>
> On 23.06.25 14:08:41, Pranav Tyagi wrote:
> > Use the scope based resource management (defined in linux/cleanup.h) to
> > automate the lifetime control of struct cxl_mbox_transfer_fw. This
> > eliminates explicit kfree() calls and makes the code more robust and
> > maintainable in presence of early returns.
> >
> > Signed-off-by: Pranav Tyagi <pranav.tyagi03@gmail.com>
> > ---
> >  drivers/cxl/core/memdev.c | 21 ++++++++-------------
> >  1 file changed, 8 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> > index f88a13adf7fa..38f4449f9740 100644
> > --- a/drivers/cxl/core/memdev.c
> > +++ b/drivers/cxl/core/memdev.c
> > @@ -7,6 +7,7 @@
> >  #include <linux/slab.h>
> >  #include <linux/idr.h>
> >  #include <linux/pci.h>
> > +#include <linux/cleanup.h>
> >  #include <cxlmem.h>
> >  #include "trace.h"
> >  #include "core.h"
> > @@ -802,11 +803,10 @@ static int cxl_mem_activate_fw(struct cxl_memdev_state *mds, int slot)
> >  static int cxl_mem_abort_fw_xfer(struct cxl_memdev_state *mds)
> >  {
> >       struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox;
> > -     struct cxl_mbox_transfer_fw *transfer;
> >       struct cxl_mbox_cmd mbox_cmd;
> > -     int rc;
> > -
> > -     transfer = kzalloc(struct_size(transfer, data, 0), GFP_KERNEL);
> > +
> > +     struct cxl_mbox_transfer_fw *transfer __free(kfree) =
> > +             kzalloc(struct_size(transfer, data, 0), GFP_KERNEL);
>
> I don't see a reason for __free() here as there are no early exits.
>
> >       if (!transfer)
> >               return -ENOMEM;
> >
> > @@ -821,9 +821,7 @@ static int cxl_mem_abort_fw_xfer(struct cxl_memdev_state *mds)
> >
> >       transfer->action = CXL_FW_TRANSFER_ACTION_ABORT;
> >
> > -     rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
> > -     kfree(transfer);
> > -     return rc;
> > +     return cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
> >  }
> >
> >  static void cxl_fw_cleanup(struct fw_upload *fwl)
> > @@ -880,7 +878,7 @@ static enum fw_upload_err cxl_fw_write(struct fw_upload *fwl, const u8 *data,
> >       struct cxl_dev_state *cxlds = &mds->cxlds;
> >       struct cxl_mailbox *cxl_mbox = &cxlds->cxl_mbox;
> >       struct cxl_memdev *cxlmd = cxlds->cxlmd;
> > -     struct cxl_mbox_transfer_fw *transfer;
> > +     struct cxl_mbox_transfer_fw *transfer __free(kfree);
>
> Jonathan already catched this.
>
> >       struct cxl_mbox_cmd mbox_cmd;
> >       u32 cur_size, remaining;
> >       size_t size_in;
> > @@ -949,7 +947,7 @@ static enum fw_upload_err cxl_fw_write(struct fw_upload *fwl, const u8 *data,
> >       rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
> >       if (rc < 0) {
> >               rc = FW_UPLOAD_ERR_RW_ERROR;
> > -             goto out_free;
> > +             return rc;
>
> If you want to remove the goto here, just free transfer right after
> calling cxl_internal_send_cmd(). It is no longer used then.
>
> I only want those cleanup helpers where they are actually useful and
> do not just add complexity.
>
> Thanks,
>
> -Robert
>
> >       }
> >
> >       *written = cur_size;
> > @@ -963,14 +961,11 @@ static enum fw_upload_err cxl_fw_write(struct fw_upload *fwl, const u8 *data,
> >                       dev_err(&cxlmd->dev, "Error activating firmware: %d\n",
> >                               rc);
> >                       rc = FW_UPLOAD_ERR_HW_ERROR;
> > -                     goto out_free;
> > +                     return rc;
> >               }
> >       }
> >
> >       rc = FW_UPLOAD_ERR_NONE;
> > -
> > -out_free:
> > -     kfree(transfer);
> >       return rc;
> >  }
> >
> > --
> > 2.49.0
> >

Thank you for the feedback. I understand your concerns and completely
agree with your reasoning. Please pardon my misjudgment in sending this
patch. I am still a beginner with kernel development and learning to
better assess what makes a meaningful contribution.

Regards
Pranav Tyagi

Re: [PATCH v2] cxl/memdev: automate cleanup with __free()

Posted by Greg KH 3 months, 2 weeks ago

On Thu, Jun 26, 2025 at 08:02:10PM +0530, Pranav Tyagi wrote:
> Thank you for the feedback. I understand your concerns and completely
> agree with your reasoning. Please pardon my misjudgment in sending this
> patch. I am still a beginner with kernel development and learning to
> better assess what makes a meaningful contribution.

I suggest you start in drivers/staging/ which is specifically designed
for beginners to get involved without having to bother other maintainers :)

thanks,

greg k-h

Re: [PATCH v2] cxl/memdev: automate cleanup with __free()

Posted by Pranav Tyagi 3 months, 2 weeks ago

On Thu, Jun 26, 2025 at 8:26 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Jun 26, 2025 at 08:02:10PM +0530, Pranav Tyagi wrote:
> > Thank you for the feedback. I understand your concerns and completely
> > agree with your reasoning. Please pardon my misjudgment in sending this
> > patch. I am still a beginner with kernel development and learning to
> > better assess what makes a meaningful contribution.
>
> I suggest you start in drivers/staging/ which is specifically designed
> for beginners to get involved without having to bother other maintainers :)
>
> thanks,
>
> greg k-h

Hi Greg,

Thank you for the suggestion. I’ll start looking into
drivers/staging/ and focus my efforts there. I appreciate your
patience and guidance.

Regards
Pranav Tyagi

Re: [PATCH v2] cxl/memdev: automate cleanup with __free()

Posted by Jonathan Cameron 3 months, 2 weeks ago

On Mon, 23 Jun 2025 14:08:41 +0530
Pranav Tyagi <pranav.tyagi03@gmail.com> wrote:

> Use the scope based resource management (defined in linux/cleanup.h) to
> automate the lifetime control of struct cxl_mbox_transfer_fw. This
> eliminates explicit kfree() calls and makes the code more robust and
> maintainable in presence of early returns.
> 
> Signed-off-by: Pranav Tyagi <pranav.tyagi03@gmail.com>

Resend as I accidentally only sent 1st reply to Pranav.

> ---
>  drivers/cxl/core/memdev.c | 21 ++++++++-------------
>  1 file changed, 8 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index f88a13adf7fa..38f4449f9740 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -7,6 +7,7 @@
>  #include <linux/slab.h>
>  #include <linux/idr.h>
>  #include <linux/pci.h>
> +#include <linux/cleanup.h>
>  #include <cxlmem.h>
>  #include "trace.h"
>  #include "core.h"
> @@ -802,11 +803,10 @@ static int cxl_mem_activate_fw(struct cxl_memdev_state *mds, int slot)
>  static int cxl_mem_abort_fw_xfer(struct cxl_memdev_state *mds)
>  {
>  	struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox;
> -	struct cxl_mbox_transfer_fw *transfer;
>  	struct cxl_mbox_cmd mbox_cmd;
> -	int rc;
> -
> -	transfer = kzalloc(struct_size(transfer, data, 0), GFP_KERNEL);
> +	
> +	struct cxl_mbox_transfer_fw *transfer __free(kfree) =
> +		kzalloc(struct_size(transfer, data, 0), GFP_KERNEL);

This one is fine.

>  	if (!transfer)
>  		return -ENOMEM;
>  
> @@ -821,9 +821,7 @@ static int cxl_mem_abort_fw_xfer(struct cxl_memdev_state *mds)
>  
>  	transfer->action = CXL_FW_TRANSFER_ACTION_ABORT;
>  
> -	rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
> -	kfree(transfer);
> -	return rc;
> +	return cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
>  }
>  
>  static void cxl_fw_cleanup(struct fw_upload *fwl)
> @@ -880,7 +878,7 @@ static enum fw_upload_err cxl_fw_write(struct fw_upload *fwl, const u8 *data,
>  	struct cxl_dev_state *cxlds = &mds->cxlds;
>  	struct cxl_mailbox *cxl_mbox = &cxlds->cxl_mbox;
>  	struct cxl_memdev *cxlmd = cxlds->cxlmd;
> -	struct cxl_mbox_transfer_fw *transfer;
> +	struct cxl_mbox_transfer_fw *transfer __free(kfree);

This one is not.

Look at the comments in cleanup.h and consider if this obeys the rules
on use of this feature that are laid out there.  As it stands you will have
kfree() of an uninitialized pointer with unpredictable results in some
of the error paths.

>  	struct cxl_mbox_cmd mbox_cmd;
>  	u32 cur_size, remaining;
>  	size_t size_in;
> @@ -949,7 +947,7 @@ static enum fw_upload_err cxl_fw_write(struct fw_upload *fwl, const u8 *data,
>  	rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
>  	if (rc < 0) {
>  		rc = FW_UPLOAD_ERR_RW_ERROR;
> -		goto out_free;
> +		return rc;
>  	}
>  
>  	*written = cur_size;
> @@ -963,14 +961,11 @@ static enum fw_upload_err cxl_fw_write(struct fw_upload *fwl, const u8 *data,
>  			dev_err(&cxlmd->dev, "Error activating firmware: %d\n",
>  				rc);
>  			rc = FW_UPLOAD_ERR_HW_ERROR;
> -			goto out_free;
> +			return rc;
>  		}
>  	}
>  
>  	rc = FW_UPLOAD_ERR_NONE;

return FW_UPLOAD_ERR_NONE;

> -
> -out_free:
> -	kfree(transfer);
>  	return rc;
>  }
>