linux-sgx.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/24] use vmalloc_array and vcalloc
@ 2023-06-27 14:43 Julia Lawall
  2023-06-27 14:43 ` [PATCH v2 21/24] x86/sgx: " Julia Lawall
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Julia Lawall @ 2023-06-27 14:43 UTC (permalink / raw)
  To: linux-hyperv
  Cc: kernel-janitors, keescook, christophe.jaillet, kuba, kasan-dev,
	Andrey Konovalov, Dmitry Vyukov, iommu, linux-tegra,
	Robin Murphy, Krishna Reddy, virtualization, Xuan Zhuo,
	linux-scsi, linaro-mm-sig, linux-media, John Stultz,
	Brian Starkey, Laura Abbott, Liam Mark, Benjamin Gaignard,
	dri-devel, linux-kernel, netdev, Shailend Chand, linux-rdma, mhi,
	linux-arm-msm, linux-btrfs, intel-gvt-dev, intel-gfx,
	Dave Hansen, H. Peter Anvin, linux-sgx

The functions vmalloc_array and vcalloc were introduced in

commit a8749a35c399 ("mm: vmalloc: introduce array allocation functions")

but are not used much yet.  This series introduces uses of
these functions, to protect against multiplication overflows.

The changes were done using the following Coccinelle semantic
patch.

@initialize:ocaml@
@@

let rename alloc =
  match alloc with
    "vmalloc" -> "vmalloc_array"
  | "vzalloc" -> "vcalloc"
  | _ -> failwith "unknown"

@@
    size_t e1,e2;
    constant C1, C2;
    expression E1, E2, COUNT, x1, x2, x3;
    typedef u8;
    typedef __u8;
    type t = {u8,__u8,char,unsigned char};
    identifier alloc = {vmalloc,vzalloc};
    fresh identifier realloc = script:ocaml(alloc) { rename alloc };
@@

(
      alloc(x1*x2*x3)
|
      alloc(C1 * C2)
|
      alloc((sizeof(t)) * (COUNT), ...)
|
-     alloc((e1) * (e2))
+     realloc(e1, e2)
|
-     alloc((e1) * (COUNT))
+     realloc(COUNT, e1)
|
-     alloc((E1) * (E2))
+     realloc(E1, E2)
)

v2: This series uses vmalloc_array and vcalloc instead of
array_size.  It also leaves a multiplication of a constant by a
sizeof as is.  Two patches are thus dropped from the series.

---

 arch/x86/kernel/cpu/sgx/main.c                    |    2 +-
 drivers/accel/habanalabs/common/device.c          |    3 ++-
 drivers/accel/habanalabs/common/state_dump.c      |    7 ++++---
 drivers/bus/mhi/host/init.c                       |    2 +-
 drivers/comedi/comedi_buf.c                       |    4 ++--
 drivers/dma-buf/heaps/system_heap.c               |    2 +-
 drivers/gpu/drm/gud/gud_pipe.c                    |    2 +-
 drivers/gpu/drm/i915/gvt/gtt.c                    |    6 ++++--
 drivers/infiniband/hw/bnxt_re/qplib_res.c         |    4 ++--
 drivers/infiniband/hw/erdma/erdma_verbs.c         |    4 ++--
 drivers/infiniband/sw/siw/siw_qp.c                |    4 ++--
 drivers/infiniband/sw/siw/siw_verbs.c             |    6 +++---
 drivers/iommu/tegra-gart.c                        |    4 ++--
 drivers/net/ethernet/amd/pds_core/core.c          |    4 ++--
 drivers/net/ethernet/freescale/enetc/enetc.c      |    4 ++--
 drivers/net/ethernet/google/gve/gve_tx.c          |    2 +-
 drivers/net/ethernet/marvell/octeon_ep/octep_rx.c |    2 +-
 drivers/net/ethernet/microsoft/mana/hw_channel.c  |    2 +-
 drivers/net/ethernet/pensando/ionic/ionic_lif.c   |    4 ++--
 drivers/scsi/fnic/fnic_trace.c                    |    2 +-
 drivers/scsi/qla2xxx/qla_init.c                   |    4 ++--
 drivers/vdpa/vdpa_user/iova_domain.c              |    4 ++--
 drivers/virtio/virtio_mem.c                       |    6 +++---
 fs/btrfs/zoned.c                                  |    4 ++--
 kernel/kcov.c                                     |    2 +-
 lib/test_vmalloc.c                                |    9 +++++----
 26 files changed, 52 insertions(+), 47 deletions(-)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 21/24] x86/sgx: use vmalloc_array and vcalloc
  2023-06-27 14:43 [PATCH v2 00/24] use vmalloc_array and vcalloc Julia Lawall
@ 2023-06-27 14:43 ` Julia Lawall
  2023-06-27 14:54   ` Dave Hansen
  2023-06-27 16:40 ` [PATCH v2 00/24] " patchwork-bot+netdevbpf
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Julia Lawall @ 2023-06-27 14:43 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: kernel-janitors, keescook, christophe.jaillet, kuba, Dave Hansen,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	H. Peter Anvin, linux-sgx, linux-kernel

Use vmalloc_array and vcalloc to protect against
multiplication overflows.

The changes were done using the following Coccinelle
semantic patch:

// <smpl>
@initialize:ocaml@
@@

let rename alloc =
  match alloc with
    "vmalloc" -> "vmalloc_array"
  | "vzalloc" -> "vcalloc"
  | _ -> failwith "unknown"

@@
    size_t e1,e2;
    constant C1, C2;
    expression E1, E2, COUNT, x1, x2, x3;
    typedef u8;
    typedef __u8;
    type t = {u8,__u8,char,unsigned char};
    identifier alloc = {vmalloc,vzalloc};
    fresh identifier realloc = script:ocaml(alloc) { rename alloc };
@@

(
      alloc(x1*x2*x3)
|
      alloc(C1 * C2)
|
      alloc((sizeof(t)) * (COUNT), ...)
|
-     alloc((e1) * (e2))
+     realloc(e1, e2)
|
-     alloc((e1) * (COUNT))
+     realloc(COUNT, e1)
|
-     alloc((E1) * (E2))
+     realloc(E1, E2)
)
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@inria.fr>

---
v2: Use vmalloc_array and vcalloc instead of array_size.
This also leaves a multiplication of a constant by a sizeof
as is.  Two patches are thus dropped from the series.

 arch/x86/kernel/cpu/sgx/main.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -u -p a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -628,7 +628,7 @@ static bool __init sgx_setup_epc_section
 	if (!section->virt_addr)
 		return false;
 
-	section->pages = vmalloc(nr_pages * sizeof(struct sgx_epc_page));
+	section->pages = vmalloc_array(nr_pages, sizeof(struct sgx_epc_page));
 	if (!section->pages) {
 		memunmap(section->virt_addr);
 		return false;


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 21/24] x86/sgx: use vmalloc_array and vcalloc
  2023-06-27 14:43 ` [PATCH v2 21/24] x86/sgx: " Julia Lawall
@ 2023-06-27 14:54   ` Dave Hansen
  2023-06-27 15:01     ` Julia Lawall
  0 siblings, 1 reply; 8+ messages in thread
From: Dave Hansen @ 2023-06-27 14:54 UTC (permalink / raw)
  To: Julia Lawall, Jarkko Sakkinen
  Cc: kernel-janitors, keescook, christophe.jaillet, kuba, Dave Hansen,
	Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	H. Peter Anvin, linux-sgx, linux-kernel

On 6/27/23 07:43, Julia Lawall wrote:
> Use vmalloc_array and vcalloc to protect against
> multiplication overflows.
...
> diff -u -p a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
> --- a/arch/x86/kernel/cpu/sgx/main.c
> +++ b/arch/x86/kernel/cpu/sgx/main.c
> @@ -628,7 +628,7 @@ static bool __init sgx_setup_epc_section
>  	if (!section->virt_addr)
>  		return false;
>  
> -	section->pages = vmalloc(nr_pages * sizeof(struct sgx_epc_page));
> +	section->pages = vmalloc_array(nr_pages, sizeof(struct sgx_epc_page));
>  	if (!section->pages) {

I'm not sure that changelog matches the code.

'nr_pages' here is an 'unsigned long' and The sizeof()==32.  In
practice, the multiplication can be done with a shift, and the ulong is
a *LONG* way from overflowing.

I'll accept that, as a general rule, vmalloc_array() is the preferred
form.  It's totally possible that someone could copy and paste the
nr_foo*sizeof(struct bar) code over to a place where nr_foo is a more
troublesome type.

But, if that's the true motivation, could we please say that in the
changelog?  As it stands, it's a bit silly to be talking about
multiplication overflows, unless I'm missing something totally obvious.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 21/24] x86/sgx: use vmalloc_array and vcalloc
  2023-06-27 14:54   ` Dave Hansen
@ 2023-06-27 15:01     ` Julia Lawall
  2023-06-27 15:06       ` Dave Hansen
  0 siblings, 1 reply; 8+ messages in thread
From: Julia Lawall @ 2023-06-27 15:01 UTC (permalink / raw)
  To: Dave Hansen
  Cc: Jarkko Sakkinen, kernel-janitors, keescook, christophe.jaillet,
	kuba, Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	x86, H. Peter Anvin, linux-sgx, linux-kernel



On Tue, 27 Jun 2023, Dave Hansen wrote:

> On 6/27/23 07:43, Julia Lawall wrote:
> > Use vmalloc_array and vcalloc to protect against
> > multiplication overflows.
> ...
> > diff -u -p a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
> > --- a/arch/x86/kernel/cpu/sgx/main.c
> > +++ b/arch/x86/kernel/cpu/sgx/main.c
> > @@ -628,7 +628,7 @@ static bool __init sgx_setup_epc_section
> >  	if (!section->virt_addr)
> >  		return false;
> >
> > -	section->pages = vmalloc(nr_pages * sizeof(struct sgx_epc_page));
> > +	section->pages = vmalloc_array(nr_pages, sizeof(struct sgx_epc_page));
> >  	if (!section->pages) {
>
> I'm not sure that changelog matches the code.
>
> 'nr_pages' here is an 'unsigned long' and The sizeof()==32.  In
> practice, the multiplication can be done with a shift, and the ulong is
> a *LONG* way from overflowing.
>
> I'll accept that, as a general rule, vmalloc_array() is the preferred
> form.  It's totally possible that someone could copy and paste the
> nr_foo*sizeof(struct bar) code over to a place where nr_foo is a more
> troublesome type.
>
> But, if that's the true motivation, could we please say that in the
> changelog?  As it stands, it's a bit silly to be talking about
> multiplication overflows, unless I'm missing something totally obvious.

If it is certain that no overflow is possible, then perhaps it is fine to
drop the patch?  I didn't change cases where both arguments are constants
nor where the result of the sizeof is 1.  But I also didn't do a careful
analysis to see if an overflow is possible given the possible values
involved.

Or if it seems better to keep the change, I can also change the log
message.

julia

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 21/24] x86/sgx: use vmalloc_array and vcalloc
  2023-06-27 15:01     ` Julia Lawall
@ 2023-06-27 15:06       ` Dave Hansen
  0 siblings, 0 replies; 8+ messages in thread
From: Dave Hansen @ 2023-06-27 15:06 UTC (permalink / raw)
  To: Julia Lawall
  Cc: Jarkko Sakkinen, kernel-janitors, keescook, christophe.jaillet,
	kuba, Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	x86, H. Peter Anvin, linux-sgx, linux-kernel

On 6/27/23 08:01, Julia Lawall wrote:
> If it is certain that no overflow is possible, then perhaps it is fine to
> drop the patch? 

It's impossible in practice in this case because the code is 64-bit only
and uses an 'unsigned long'.  But, like I said, I can see that same
vmalloc() being copied-and-pasted or moved to a 32-bit system and
theoretically causing problems in rare scenarios.

I'd probably just drop this patch.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 00/24] use vmalloc_array and vcalloc
  2023-06-27 14:43 [PATCH v2 00/24] use vmalloc_array and vcalloc Julia Lawall
  2023-06-27 14:43 ` [PATCH v2 21/24] x86/sgx: " Julia Lawall
@ 2023-06-27 16:40 ` patchwork-bot+netdevbpf
  2023-07-06  1:35 ` Martin K. Petersen
  2023-07-11 16:31 ` (subset) " Martin K. Petersen
  3 siblings, 0 replies; 8+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-06-27 16:40 UTC (permalink / raw)
  To: Julia Lawall
  Cc: linux-hyperv, kernel-janitors, keescook, christophe.jaillet,
	kuba, kasan-dev, andreyknvl, dvyukov, iommu, linux-tegra,
	robin.murphy, vdumpa, virtualization, xuanzhuo, linux-scsi,
	linaro-mm-sig, linux-media, jstultz, Brian.Starkey, labbott,
	lmark, benjamin.gaignard, dri-devel, linux-kernel, netdev,
	shailend, linux-rdma, mhi, linux-arm-msm, linux-btrfs,
	intel-gvt-dev, intel-gfx, dave.hansen, hpa, linux-sgx

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 27 Jun 2023 16:43:15 +0200 you wrote:
> The functions vmalloc_array and vcalloc were introduced in
> 
> commit a8749a35c399 ("mm: vmalloc: introduce array allocation functions")
> 
> but are not used much yet.  This series introduces uses of
> these functions, to protect against multiplication overflows.
> 
> [...]

Here is the summary with links:
  - [v2,02/24] octeon_ep: use vmalloc_array and vcalloc
    https://git.kernel.org/netdev/net-next/c/32d462a5c3e5
  - [v2,04/24] gve: use vmalloc_array and vcalloc
    https://git.kernel.org/netdev/net-next/c/a13de901e8d5
  - [v2,09/24] pds_core: use vmalloc_array and vcalloc
    https://git.kernel.org/netdev/net-next/c/906a76cc7645
  - [v2,11/24] ionic: use vmalloc_array and vcalloc
    https://git.kernel.org/netdev/net-next/c/f712c8297e0a
  - [v2,18/24] net: enetc: use vmalloc_array and vcalloc
    https://git.kernel.org/netdev/net-next/c/fa87c54693ae
  - [v2,22/24] net: mana: use vmalloc_array and vcalloc
    https://git.kernel.org/netdev/net-next/c/e9c74f8b8a31

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 00/24] use vmalloc_array and vcalloc
  2023-06-27 14:43 [PATCH v2 00/24] use vmalloc_array and vcalloc Julia Lawall
  2023-06-27 14:43 ` [PATCH v2 21/24] x86/sgx: " Julia Lawall
  2023-06-27 16:40 ` [PATCH v2 00/24] " patchwork-bot+netdevbpf
@ 2023-07-06  1:35 ` Martin K. Petersen
  2023-07-11 16:31 ` (subset) " Martin K. Petersen
  3 siblings, 0 replies; 8+ messages in thread
From: Martin K. Petersen @ 2023-07-06  1:35 UTC (permalink / raw)
  To: Julia Lawall
  Cc: linux-hyperv, kernel-janitors, keescook, christophe.jaillet,
	kuba, kasan-dev, Andrey Konovalov, Dmitry Vyukov, iommu,
	linux-tegra, Robin Murphy, Krishna Reddy, virtualization,
	Xuan Zhuo, linux-scsi, linaro-mm-sig, linux-media, John Stultz,
	Brian Starkey, Laura Abbott, Liam Mark, Benjamin Gaignard,
	dri-devel, linux-kernel, netdev, Shailend Chand, linux-rdma, mhi,
	linux-arm-msm, linux-btrfs, intel-gvt-dev, intel-gfx,
	Dave Hansen, H. Peter Anvin, linux-sgx


Julia,

> The functions vmalloc_array and vcalloc were introduced in
>
> commit a8749a35c399 ("mm: vmalloc: introduce array allocation functions")
>
> but are not used much yet.  This series introduces uses of
> these functions, to protect against multiplication overflows.

Applied #7 and #24 to 6.5/scsi-staging, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: (subset) [PATCH v2 00/24] use vmalloc_array and vcalloc
  2023-06-27 14:43 [PATCH v2 00/24] use vmalloc_array and vcalloc Julia Lawall
                   ` (2 preceding siblings ...)
  2023-07-06  1:35 ` Martin K. Petersen
@ 2023-07-11 16:31 ` Martin K. Petersen
  3 siblings, 0 replies; 8+ messages in thread
From: Martin K. Petersen @ 2023-07-11 16:31 UTC (permalink / raw)
  To: linux-hyperv, Julia Lawall
  Cc: Martin K . Petersen, kernel-janitors, keescook,
	christophe.jaillet, kuba, kasan-dev, Andrey Konovalov,
	Dmitry Vyukov, iommu, linux-tegra, Robin Murphy, Krishna Reddy,
	virtualization, Xuan Zhuo, linux-scsi, linaro-mm-sig,
	linux-media, John Stultz, Brian Starkey, Laura Abbott, Liam Mark,
	Benjamin Gaignard, dri-devel, linux-kernel, netdev,
	Shailend Chand, linux-rdma, mhi, linux-arm-msm, linux-btrfs,
	intel-gvt-dev, intel-gfx, Dave Hansen, H. Peter Anvin, linux-sgx

On Tue, 27 Jun 2023 16:43:15 +0200, Julia Lawall wrote:

> The functions vmalloc_array and vcalloc were introduced in
> 
> commit a8749a35c399 ("mm: vmalloc: introduce array allocation functions")
> 
> but are not used much yet.  This series introduces uses of
> these functions, to protect against multiplication overflows.
> 
> [...]

Applied to 6.5/scsi-fixes, thanks!

[07/24] scsi: fnic: use vmalloc_array and vcalloc
        https://git.kernel.org/mkp/scsi/c/b34c7dcaf311
[24/24] scsi: qla2xxx: use vmalloc_array and vcalloc
        https://git.kernel.org/mkp/scsi/c/04d91b783acf

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-07-11 16:33 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-27 14:43 [PATCH v2 00/24] use vmalloc_array and vcalloc Julia Lawall
2023-06-27 14:43 ` [PATCH v2 21/24] x86/sgx: " Julia Lawall
2023-06-27 14:54   ` Dave Hansen
2023-06-27 15:01     ` Julia Lawall
2023-06-27 15:06       ` Dave Hansen
2023-06-27 16:40 ` [PATCH v2 00/24] " patchwork-bot+netdevbpf
2023-07-06  1:35 ` Martin K. Petersen
2023-07-11 16:31 ` (subset) " Martin K. Petersen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).