[char-misc-next v3] mei: use kvmalloc for read buffer

Alexander Usyskin posted 1 patch 1 month, 1 week ago
drivers/misc/mei/client.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[char-misc-next v3] mei: use kvmalloc for read buffer
Posted by Alexander Usyskin 1 month, 1 week ago
Read buffer is allocated according to max message size, reported by
the firmware and may reach 64K in systems with pxp client.
Contiguous 64k allocation may fail under memory pressure.
Read buffer is used as in-driver message storage and not required
to be contiguous.
Use kvmalloc to allow kernel to allocate non-contiguous memory.

Fixes: 3030dc056459 ("mei: add wrapper for queuing control commands.")
Reported-by: Rohit Agarwal <rohiagar@chromium.org>
Closes: https://lore.kernel.org/all/20240813084542.2921300-1-rohiagar@chromium.org/
Tested-by: Brian Geffon <bgeffon@google.com>
Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
---

Changes since V2:
 - add Fixes and CC:stable

Changes since V1:
 - add Tested-by and Reported-by

 drivers/misc/mei/client.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index 9d090fa07516..be011cef12e5 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -321,7 +321,7 @@ void mei_io_cb_free(struct mei_cl_cb *cb)
 		return;
 
 	list_del(&cb->list);
-	kfree(cb->buf.data);
+	kvfree(cb->buf.data);
 	kfree(cb->ext_hdr);
 	kfree(cb);
 }
@@ -497,7 +497,7 @@ struct mei_cl_cb *mei_cl_alloc_cb(struct mei_cl *cl, size_t length,
 	if (length == 0)
 		return cb;
 
-	cb->buf.data = kmalloc(roundup(length, MEI_SLOT_SIZE), GFP_KERNEL);
+	cb->buf.data = kvmalloc(roundup(length, MEI_SLOT_SIZE), GFP_KERNEL);
 	if (!cb->buf.data) {
 		mei_io_cb_free(cb);
 		return NULL;
-- 
2.43.0
Re: [char-misc-next v3] mei: use kvmalloc for read buffer
Posted by Greg Kroah-Hartman 1 month, 1 week ago
On Tue, Oct 15, 2024 at 03:31:57PM +0300, Alexander Usyskin wrote:
> Read buffer is allocated according to max message size, reported by
> the firmware and may reach 64K in systems with pxp client.
> Contiguous 64k allocation may fail under memory pressure.
> Read buffer is used as in-driver message storage and not required
> to be contiguous.
> Use kvmalloc to allow kernel to allocate non-contiguous memory.
> 
> Fixes: 3030dc056459 ("mei: add wrapper for queuing control commands.")
> Reported-by: Rohit Agarwal <rohiagar@chromium.org>
> Closes: https://lore.kernel.org/all/20240813084542.2921300-1-rohiagar@chromium.org/
> Tested-by: Brian Geffon <bgeffon@google.com>
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> ---

Why is this on the -next branch?  You want this merged now, right?

Again, I asked "why hasn't this been reviewed by others at Intel", and
so I'm just going to delete this series until it has followed the
correct Intel-internal review process.

greg k-h
Re: [char-misc-next v3] mei: use kvmalloc for read buffer
Posted by Brian Geffon 1 month ago
On Tue, Oct 15, 2024 at 8:48 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Tue, Oct 15, 2024 at 03:31:57PM +0300, Alexander Usyskin wrote:
> > Read buffer is allocated according to max message size, reported by
> > the firmware and may reach 64K in systems with pxp client.
> > Contiguous 64k allocation may fail under memory pressure.
> > Read buffer is used as in-driver message storage and not required
> > to be contiguous.
> > Use kvmalloc to allow kernel to allocate non-contiguous memory.
> >
> > Fixes: 3030dc056459 ("mei: add wrapper for queuing control commands.")
> > Reported-by: Rohit Agarwal <rohiagar@chromium.org>
> > Closes: https://lore.kernel.org/all/20240813084542.2921300-1-rohiagar@chromium.org/
> > Tested-by: Brian Geffon <bgeffon@google.com>
> > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
Cc: stable@vger.kernel.org

> > ---
>
> Why is this on the -next branch?  You want this merged now, right?
>
> Again, I asked "why hasn't this been reviewed by others at Intel", and
> so I'm just going to delete this series until it has followed the
> correct Intel-internal review process.

This is a significant crash for us, any chance we can get another
reviewer from Intel?

>
> greg k-h

Thanks
Brian
Re: [char-misc-next v3] mei: use kvmalloc for read buffer
Posted by Tomas Winkler 1 month ago
On Mon, Oct 21, 2024 at 5:53 PM Brian Geffon <bgeffon@google.com> wrote:
>
> On Tue, Oct 15, 2024 at 8:48 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Oct 15, 2024 at 03:31:57PM +0300, Alexander Usyskin wrote:
> > > Read buffer is allocated according to max message size, reported by
> > > the firmware and may reach 64K in systems with pxp client.
> > > Contiguous 64k allocation may fail under memory pressure.
> > > Read buffer is used as in-driver message storage and not required
> > > to be contiguous.
> > > Use kvmalloc to allow kernel to allocate non-contiguous memory.
> > >
> > > Fixes: 3030dc056459 ("mei: add wrapper for queuing control commands.")
> > > Reported-by: Rohit Agarwal <rohiagar@chromium.org>
> > > Closes: https://lore.kernel.org/all/20240813084542.2921300-1-rohiagar@chromium.org/
> > > Tested-by: Brian Geffon <bgeffon@google.com>
> > > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> Cc: stable@vger.kernel.org
>
> > > ---
> >
> > Why is this on the -next branch?  You want this merged now, right?
> >
> > Again, I asked "why hasn't this been reviewed by others at Intel", and
> > so I'm just going to delete this series until it has followed the
> > correct Intel-internal review process.
>
> This is a significant crash for us, any chance we can get another
> reviewer from Intel?
>
>
> reviewer from Intel?

(second attempt, forgot to remove html formatting)
I'm no longer with Intel but I'm aware of this fix, so as the former
driver maintainer:
Acked-by: Tomas Winkler <tomasw@gmail.com>