Received-SPF: pass (google.com: domain of linux-kernel+bounces-128351-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Tue, 2 Apr 2024 09:25:55 -0700
From: Ira Weiny <ira.weiny@intel.com>
To: Kwangjin Ko <kwangjin.ko@sk.com>, <dave@stgolabs.net>,
	<jonathan.cameron@huawei.com>, <dave.jiang@intel.com>,
	<alison.schofield@intel.com>, <vishal.l.verma@intel.com>,
	<ira.weiny@intel.com>, <dan.j.williams@intel.com>
CC: <linux-cxl@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<kernel_team@skhynix.com>
Subject: Re: [PATCH v2 1/1] cxl/core: Fix initialization of mbox_cmd.size_out
 in get event
Message-ID: <660c3192f30ba_7cf1a29410@iweiny-mobl.notmuch>
References: <20240402081404.1106-1-kwangjin.ko@sk.com>
 <20240402081404.1106-2-kwangjin.ko@sk.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240402081404.1106-2-kwangjin.ko@sk.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?Js0AAYUvNzm0NE3dzp7wOabXwW6navsbW96mA+Jk2y/oJxKwxBikXMy1ivcx?=
 =?us-ascii?Q?M+Iv1NOL0OjYZWj0lamv9aRqZuMQrqmjhKZNQPjT7XCBfY+Nfwws+pwVT9xS?=
 =?us-ascii?Q?iz0NMY5rnUwdkquw7rHf4qJACfHnozOxbyY0+KZ7OPXVjwDTcw+bSciOXwGs?=
 =?us-ascii?Q?hnp5X4vmxcf+wFdadhu3kS/2rl2X3yV6RuW4XN4vnRemZLjAz2rfnwItwkft?=
 =?us-ascii?Q?7g/CJ5fXOqNefFdiXaLtDeDpMnHdr5tqOR5uKqyMYYbL6Ai+LuBHbtofnMWa?=
 =?us-ascii?Q?rVVIpp4akbVROLWUf03xbMdrgaK4xO/KBPDPBKk57rG+Wcsz3hJX65UmLrkE?=
 =?us-ascii?Q?17oXxOX/c0P2zlDr3CXulR8xwchbRaOU1FgacN71ma7+xF8ay/7w3a2d216M?=
 =?us-ascii?Q?ef2vV9xIQqGSHlSsgJCROgrcgZ0F5eUiRDggFa9BgvfKQJ5lDech/tKjQLIQ?=
 =?us-ascii?Q?g9X18VhyItpJa6RvrofLnXh1Ok/KqVBFwc09wqLEVQFEfxujP+jvAoBYQkOe?=
 =?us-ascii?Q?osfFEj85xD2xMrEoLNIVtiO6HcrdpoxH2zXCbkagZyFjBwJb0DnzuYO6TJUK?=
 =?us-ascii?Q?N7+pWIEalFqVEVMLzXKJM9ZCMS9qGMBV4OZYA7oyk21Sjl/6mQqUMzJEvop0?=
 =?us-ascii?Q?Csmat8YI5SO6G+Tccy09qqsVJIbW5ZUqHLCrVpQ+d/MDZt+IboWkBhUfZIHB?=
 =?us-ascii?Q?LRMK1wkskcz2PRPwNqK+AUoRY93xSoca6YEEEix4T/yUqrku601d+XjoPb70?=
 =?us-ascii?Q?4AS36kfa0t4tsCSpOcy4oAltQx60DtGleYyKTrunXdhbBUDpjs7PSqSNSKUY?=
 =?us-ascii?Q?dz9V4lNFXq4DrKX1bLFbPVIpqvva9fTmA08zXr2yXSG+IdlSJyqCXmzE9AFP?=
 =?us-ascii?Q?Rrd9VhTu64xPGv4GpdCgp3zPVZW86WhIB3x46p1DIv+KpzM3PP6Bd6grn1fx?=
 =?us-ascii?Q?ylq0mdkNCl4Ahy+Uxn4hpEI72jHLuGVV5YNlb1DGftr+nrtPIaxZVKdjhPly?=
 =?us-ascii?Q?0o0WjET0eY7vUglffSvul2PXfVNcySvDnCMPsMEtLQCMOneNEkLo3Pve2Zsj?=
 =?us-ascii?Q?lpcW6d1IhD23iZR9TENHIzsxk4HiQcxf6Cif8jmHCCwIXKKIRHzZOYhhwyl4?=
 =?us-ascii?Q?D+kLGydCZf0m43uKaHUMPeCi/Wi/D11apncdnAznAbeRTsVYXJQy1iiiTl0n?=
 =?us-ascii?Q?H7hxQOiKSTtwQFI4yLIf4YIxnLVmxoD/QjTMFaVn9gZK9njaq22FhCB6mq7M?=
 =?us-ascii?Q?sED9k/evXyVZmrai/ehB4gU9EcBbrj3BUnCVaVBgrW0OFwKhNJjiTxvqRSAT?=
 =?us-ascii?Q?+uBmNZy5NojB+UPaMUikQgVqzheKWMu6P7Btwu6a/5szRU4swQRCjhLZMbOT?=
 =?us-ascii?Q?iGjOVqieDJrkoBiYA3rzSdCrC5a0MESrHS8nm7jQ80rORQrxK115df8G4jRn?=
 =?us-ascii?Q?r6Vfpr5Yg+tJ5uTRe7Tu2XOQqD3qw/06hhX5DFrjaH+xOOcywRm1r3FoduCu?=
 =?us-ascii?Q?a0YbsPeQidISxv7siqiVU3kaFg8AjyugjpuU23KY2eZnWdOO85+rTVZ5G5St?=
 =?us-ascii?Q?VpiEN2pmjme2sfjhkhcUvuz1hl2msUL7egF/p30f?=
X-MS-Exchange-CrossTenant-Network-Message-Id: f56999f1-0048-4dc8-7468-08dc533193dd
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6733.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Apr 2024 16:25:58.1235
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: dWTQIDGyOVFdvLNP/DtySPfcYURyxTMyLCyd/zW6jZmc0d6xkEQDXFlB/RoQqU4P/+FZ34t7d2bmlXobnphiLw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR11MB6889
X-OriginatorOrg: intel.com

Kwangjin Ko wrote:
> Since mbox_cmd.size_out is overwritten with the actual output size in
> the function below, it needs to be initialized every time.
> 
> cxl_internal_send_cmd -> __cxl_pci_mbox_send_cmd
> 
> Problem scenario:
> 
> 1) The size_out variable is initially set to the size of the mailbox.
> 2) Read an event.
>    - size_out is set to 160 bytes(header 32B + one event 128B).
>    - Two event are created while reading.
> 3) Read the new *two* events.
>    - size_out is still set to 160 bytes.
>    - Although the value of out_len is 288 bytes, only 160 bytes are
>      copied from the mailbox register to the local variable.
>    - record_count is set to 2.
>    - Accessing records[1] will result in reading incorrect data.
> 
> Signed-off-by: Kwangjin Ko <kwangjin.ko@sk.com>


Tested-by: Ira Weiny <ira.weiny@intel.com>
Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> ---
>  drivers/cxl/core/mbox.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 9adda4795eb7..a38531a055c8 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -958,13 +958,14 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
>  		.payload_in = &log_type,
>  		.size_in = sizeof(log_type),
>  		.payload_out = payload,
> -		.size_out = mds->payload_size,
>  		.min_out = struct_size(payload, records, 0),
>  	};
>  
>  	do {
>  		int rc, i;
>  
> +		mbox_cmd.size_out = mds->payload_size;
> +
>  		rc = cxl_internal_send_cmd(mds, &mbox_cmd);
>  		if (rc) {
>  			dev_err_ratelimited(dev,
> -- 
> 2.34.1
>