Re: [PATCH][next] mm/vmalloc: Fix read of pointer area after it has been free'd
On Mon, Mar 29, 2021 at 06:14:34PM +0100, Matthew Wilcox wrote:
> On Mon, Mar 29, 2021 at 06:07:30PM +0100, Colin King wrote:
> > From: Colin Ian King
> >
> > Currently the memory pointed to by area is being freed by the
> > free_vm_area call and then area->nr_pages is referencing the
> > free'd object. Fix this swapping the order of the warn_alloc
> > message and the free.
> >
> > Addresses-Coverity: ("Read from pointer after free")
> > Fixes: 014ccf9b888d ("mm/vmalloc: improve allocation failure error
> > messages")
>
> i don't have this git sha. if this is -next, the sha ids aren't stable
> and shouldn't be referenced in commit logs, because these fixes should
> just be squashed into the not-yet-upstream commits.
The fixes tag won't "be referenced in the commit logs" because Andew
is going to fold it in. And the "mm/vmalloc: improve allocation failure
error messages" information is useful.
More generally there are a bunch trees which rebase often but it's
humanly impossible to remember which ones they are and there is no
automated way to track it either. If the maintainer is going to rebase
then they're going to have to think about Fixes tags a bit. Normally
rebased trees fold fixes so it's not an issue. Otherwise it would be
easy to write a script to correct the Fixes tags during a rebase.
In other words, my view is that everyone should just use the same Fixes
tag format everywhere.
regards,
dan carpenter
Re: [PATCH][next] mm/vmalloc: Fix read of pointer area after it has been free'd
> On Mon, Mar 29, 2021 at 08:14:53PM +0200, Uladzislau Rezki wrote:
> > On Mon, Mar 29, 2021 at 07:40:29PM +0200, Uladzislau Rezki wrote:
> > > On Mon, Mar 29, 2021 at 06:14:34PM +0100, Matthew Wilcox wrote:
> > > > On Mon, Mar 29, 2021 at 06:07:30PM +0100, Colin King wrote:
> > > > > From: Colin Ian King
> > > > >
> > > > > Currently the memory pointed to by area is being freed by the
> > > > > free_vm_area call and then area->nr_pages is referencing the
> > > > > free'd object. Fix this swapping the order of the warn_alloc
> > > > > message and the free.
> > > > >
> > > > > Addresses-Coverity: ("Read from pointer after free")
> > > > > Fixes: 014ccf9b888d ("mm/vmalloc: improve allocation failure error
> > > > > messages")
> > > >
> > > > i don't have this git sha. if this is -next, the sha ids aren't stable
> > > > and shouldn't be referenced in commit logs, because these fixes should
> > > > just be squashed into the not-yet-upstream commits.
> > > >
> > > > > Signed-off-by: Colin Ian King
> > > > > ---
> > > > > mm/vmalloc.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > > > > index b73e4e715e0d..7936405749e4 100644
> > > > > --- a/mm/vmalloc.c
> > > > > +++ b/mm/vmalloc.c
> > > > > @@ -2790,11 +2790,11 @@ static void *__vmalloc_area_node(struct
> > > > > vm_struct *area, gfp_t gfp_mask,
> > > > > }
> > > > >
> > > > > if (!pages) {
> > > > > - free_vm_area(area);
> > > > > warn_alloc(gfp_mask, NULL,
> > > > > "vmalloc size %lu allocation failure: "
> > > > > "page array size %lu allocation failed",
> > > > > area->nr_pages * PAGE_SIZE, array_size);
> > > > > + free_vm_area(area);
> > > > > return NULL;
> > > >
> > > > this fix looks right to me.
> > > >
> > > That is from the linux-next. Same to me.
> > >
> > > Reviewed-by: Uladzislau Rezki (Sony)
> > >
> > > --
> > > Vlad Rezki
> > Is the linux-next(next-20210329) broken?
> >
> Please ignore my previous email. That was due to my local "stashed" change.
>
Hello, Andrew.
Could you please squash below patch with the one that is in question?
Or should i send out it as separate patch?
>From 6d1c221fec4718094c6e825e3879a76ad70dba93 Mon Sep 17 00:00:00 2001
From: "Uladzislau Rezki (Sony)"
Date: Mon, 29 Mar 2021 21:12:47 +0200
Subject: [PATCH] mm/vmalloc: print correct vmalloc allocation size
On entry the area->nr_pages is not set yet and is zero, thus
when an allocation of the page-table array fails the vmalloc
size will not be reflected correctly in a error message.
Replace area->nr_pages by the nr_small_pages.
Fixes: 014ccf9b888d ("mm/vmalloc: improve allocation failure error messages")
Signed-off-by: Uladzislau Rezki (Sony)
---
mm/vmalloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index b73e4e715e0d..8b564f91a610 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -2794,7 +2794,7 @@ static void *__vmalloc_area_node(struct vm_struct *area,
gfp_t gfp_mask,
warn_alloc(gfp_mask, NULL,
"vmalloc size %lu allocation failure: "
"page array size %lu allocation failed",
- area->nr_pages * PAGE_SIZE, array_size);
+ nr_small_pages * PAGE_SIZE, array_size);
return NULL;
}
--
2.20.1
--
Vlad Rezki
Re: [PATCH][next] mm/vmalloc: Fix read of pointer area after it has been free'd
On Mon, Mar 29, 2021 at 08:14:53PM +0200, Uladzislau Rezki wrote:
> On Mon, Mar 29, 2021 at 07:40:29PM +0200, Uladzislau Rezki wrote:
> > On Mon, Mar 29, 2021 at 06:14:34PM +0100, Matthew Wilcox wrote:
> > > On Mon, Mar 29, 2021 at 06:07:30PM +0100, Colin King wrote:
> > > > From: Colin Ian King
> > > >
> > > > Currently the memory pointed to by area is being freed by the
> > > > free_vm_area call and then area->nr_pages is referencing the
> > > > free'd object. Fix this swapping the order of the warn_alloc
> > > > message and the free.
> > > >
> > > > Addresses-Coverity: ("Read from pointer after free")
> > > > Fixes: 014ccf9b888d ("mm/vmalloc: improve allocation failure error
> > > > messages")
> > >
> > > i don't have this git sha. if this is -next, the sha ids aren't stable
> > > and shouldn't be referenced in commit logs, because these fixes should
> > > just be squashed into the not-yet-upstream commits.
> > >
> > > > Signed-off-by: Colin Ian King
> > > > ---
> > > > mm/vmalloc.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > > > index b73e4e715e0d..7936405749e4 100644
> > > > --- a/mm/vmalloc.c
> > > > +++ b/mm/vmalloc.c
> > > > @@ -2790,11 +2790,11 @@ static void *__vmalloc_area_node(struct
> > > > vm_struct *area, gfp_t gfp_mask,
> > > > }
> > > >
> > > > if (!pages) {
> > > > - free_vm_area(area);
> > > > warn_alloc(gfp_mask, NULL,
> > > >"vmalloc size %lu allocation failure: "
> > > >"page array size %lu allocation failed",
> > > >area->nr_pages * PAGE_SIZE, array_size);
> > > > + free_vm_area(area);
> > > > return NULL;
> > >
> > > this fix looks right to me.
> > >
> > That is from the linux-next. Same to me.
> >
> > Reviewed-by: Uladzislau Rezki (Sony)
> >
> > --
> > Vlad Rezki
> Is the linux-next(next-20210329) broken?
>
Please ignore my previous email. That was due to my local "stashed" change.
--
Vlad Rezki
Re: [PATCH][next] mm/vmalloc: Fix read of pointer area after it has been free'd
On Mon, Mar 29, 2021 at 07:40:29PM +0200, Uladzislau Rezki wrote:
> On Mon, Mar 29, 2021 at 06:14:34PM +0100, Matthew Wilcox wrote:
> > On Mon, Mar 29, 2021 at 06:07:30PM +0100, Colin King wrote:
> > > From: Colin Ian King
> > >
> > > Currently the memory pointed to by area is being freed by the
> > > free_vm_area call and then area->nr_pages is referencing the
> > > free'd object. Fix this swapping the order of the warn_alloc
> > > message and the free.
> > >
> > > Addresses-Coverity: ("Read from pointer after free")
> > > Fixes: 014ccf9b888d ("mm/vmalloc: improve allocation failure error
> > > messages")
> >
> > i don't have this git sha. if this is -next, the sha ids aren't stable
> > and shouldn't be referenced in commit logs, because these fixes should
> > just be squashed into the not-yet-upstream commits.
> >
> > > Signed-off-by: Colin Ian King
> > > ---
> > > mm/vmalloc.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > > index b73e4e715e0d..7936405749e4 100644
> > > --- a/mm/vmalloc.c
> > > +++ b/mm/vmalloc.c
> > > @@ -2790,11 +2790,11 @@ static void *__vmalloc_area_node(struct vm_struct
> > > *area, gfp_t gfp_mask,
> > > }
> > >
> > > if (!pages) {
> > > - free_vm_area(area);
> > > warn_alloc(gfp_mask, NULL,
> > > "vmalloc size %lu allocation failure: "
> > > "page array size %lu allocation failed",
> > > area->nr_pages * PAGE_SIZE, array_size);
> > > + free_vm_area(area);
> > > return NULL;
> >
> > this fix looks right to me.
> >
> That is from the linux-next. Same to me.
>
> Reviewed-by: Uladzislau Rezki (Sony)
>
> --
> Vlad Rezki
Is the linux-next(next-20210329) broken?
@pc638:~/data/raid0/coding/linux-next.git$ git branch
master
next-20210225
next-20210326
* next-20210329
urezki@pc638:~/data/raid0/coding/linux-next.git$ ../run_linux.sh
./arch/x86_64/boot/bzImage
File ‘quantal-trinity-x86_64.cgz’ already there; not retrieving.
early console in setup code
Probing EDD (edd=off to disable)... ok
[0.00] Linux version 5.12.0-rc4-next-20210329+ (urezki@pc638) (gcc
(Debian 8.3.0-6) 8.3.0, GNU ld (GNU Binutils for Debian) 2.31.1) #497 SMP
PREEMPT Mon Mar 29 19:59:25 CEST 2021
[0.00] Command line: root=/dev/ram0 hung_task_panic=1 debug apic=debug
sysrq_always_enabled rcupdate.rcu_cpu_stall_timeout=100 net.ifnames=0
printk.devkmsg=on panic=-1 softlockup_panic=1 nmi_watchdog=panic oops=panic
load_ramdisk=2 prompt_ramdisk=0 drbd.minor_count=8 systemd.log_level=err
ignore_loglevel console=tty0 earlyprintk=ttyS0,115200 console=ttyS0,115200
vga=normal rw rcuperf.shutdown=0 watchdog_thresh=60 run_self_test=1 threadirqs
[0.00] x86/fpu: x87 FPU will use FXSAVE
[0.00] BIOS-provided physical RAM map:
[0.00] BIOS-e820: [mem 0x-0x0009fbff] usable
[0.00] BIOS-e820: [mem 0x0009fc00-0x0009] reserved
[0.00] BIOS-e820: [mem 0x000f-0x000f] reserved
[0.00] BIOS-e820: [mem 0x0010-0xbffd] usable
[0.00] BIOS-e820: [mem 0xbffe-0xbfff] reserved
[0.00] BIOS-e820: [mem 0xfeffc000-0xfeff] reserved
[0.00] BIOS-e820: [mem 0xfffc-0x] reserved
[0.00] BIOS-e820: [mem 0x0001-0x00023fff] usable
[0.00] printk: debug: ignoring loglevel setting.
[0.00] printk: bootconsole [earlyser0] enabled
[0.00] NX (Execute Disable) protection: active
[0.00] SMBIOS 2.8 present.
[0.00] DMI: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1
04/01/2014
[0.00] Hypervisor detected: KVM
[0.00] kvm-clock: Using msrs 4b564d01 and 4b564d00
[0.00] kvm-clock: cpu 0, msr 1e96be001, primary cpu clock
[0.00] kvm-clock: using sched offset of 1302025291 cycles
[0.000492] clocksource: kvm-clock: mask: 0x max_cycles:
0x1cd42e4dffb, max_idle_ns: 881590591483 ns
[0.001870] tsc: Detected 3700.134 MHz processor
[0.002721] e820: update [mem 0x-0x0fff] usable ==> reserved
[0.003285] e820: remove [mem 0x000a-0x000f] usable
[0.028860] AGP: No AGP bridge found
[0.029382] last_pfn = 0x24 max_arch_pfn = 0x4
[0.029868] x86/PAT: Configuration [0-7]: WB WC UC- UC WB WP UC- WT
Memory KASLR using RDTSC...
[0.030657] last_pfn = 0xbffe0 max_arch_pfn = 0x4
[0.031108] Scan for SMP in [mem 0x-0x03ff]
[0.031555] Scan for SMP in [mem 0x0009fc00-0x0009]
[0.031999] Scan for SMP in [mem 0x000f-0x000f]
[0.034146] found SMP MP-table at [mem 0x000f5a80-0x000f5a8f]
[0.034622] mpc: f5a90-f5b74
[0.034881] ACPI: Early table checksum verification disabled
[0.035349] ACPI: RSDP 0x000F5850 14
Re: [PATCH][next] mm/vmalloc: Fix read of pointer area after it has been free'd
On Mon, Mar 29, 2021 at 06:14:34PM +0100, Matthew Wilcox wrote:
> On Mon, Mar 29, 2021 at 06:07:30PM +0100, Colin King wrote:
> > From: Colin Ian King
> >
> > Currently the memory pointed to by area is being freed by the
> > free_vm_area call and then area->nr_pages is referencing the
> > free'd object. Fix this swapping the order of the warn_alloc
> > message and the free.
> >
> > Addresses-Coverity: ("Read from pointer after free")
> > Fixes: 014ccf9b888d ("mm/vmalloc: improve allocation failure error
> > messages")
>
> i don't have this git sha. if this is -next, the sha ids aren't stable
> and shouldn't be referenced in commit logs, because these fixes should
> just be squashed into the not-yet-upstream commits.
>
> > Signed-off-by: Colin Ian King
> > ---
> > mm/vmalloc.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > index b73e4e715e0d..7936405749e4 100644
> > --- a/mm/vmalloc.c
> > +++ b/mm/vmalloc.c
> > @@ -2790,11 +2790,11 @@ static void *__vmalloc_area_node(struct vm_struct
> > *area, gfp_t gfp_mask,
> > }
> >
> > if (!pages) {
> > - free_vm_area(area);
> > warn_alloc(gfp_mask, NULL,
> >"vmalloc size %lu allocation failure: "
> >"page array size %lu allocation failed",
> >area->nr_pages * PAGE_SIZE, array_size);
> > + free_vm_area(area);
> > return NULL;
>
> this fix looks right to me.
>
That is from the linux-next. Same to me.
Reviewed-by: Uladzislau Rezki (Sony)
--
Vlad Rezki
Re: [PATCH][next] mm/vmalloc: Fix read of pointer area after it has been free'd
On Mon, Mar 29, 2021 at 06:07:30PM +0100, Colin King wrote:
> From: Colin Ian King
>
> Currently the memory pointed to by area is being freed by the
> free_vm_area call and then area->nr_pages is referencing the
> free'd object. Fix this swapping the order of the warn_alloc
> message and the free.
>
> Addresses-Coverity: ("Read from pointer after free")
> Fixes: 014ccf9b888d ("mm/vmalloc: improve allocation failure error messages")
i don't have this git sha. if this is -next, the sha ids aren't stable
and shouldn't be referenced in commit logs, because these fixes should
just be squashed into the not-yet-upstream commits.
> Signed-off-by: Colin Ian King
> ---
> mm/vmalloc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index b73e4e715e0d..7936405749e4 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -2790,11 +2790,11 @@ static void *__vmalloc_area_node(struct vm_struct
> *area, gfp_t gfp_mask,
> }
>
> if (!pages) {
> - free_vm_area(area);
> warn_alloc(gfp_mask, NULL,
> "vmalloc size %lu allocation failure: "
> "page array size %lu allocation failed",
> area->nr_pages * PAGE_SIZE, array_size);
> + free_vm_area(area);
> return NULL;
this fix looks right to me.
[PATCH][next] mm/vmalloc: Fix read of pointer area after it has been free'd
From: Colin Ian King
Currently the memory pointed to by area is being freed by the
free_vm_area call and then area->nr_pages is referencing the
free'd object. Fix this swapping the order of the warn_alloc
message and the free.
Addresses-Coverity: ("Read from pointer after free")
Fixes: 014ccf9b888d ("mm/vmalloc: improve allocation failure error messages")
Signed-off-by: Colin Ian King
---
mm/vmalloc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index b73e4e715e0d..7936405749e4 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -2790,11 +2790,11 @@ static void *__vmalloc_area_node(struct vm_struct
*area, gfp_t gfp_mask,
}
if (!pages) {
- free_vm_area(area);
warn_alloc(gfp_mask, NULL,
"vmalloc size %lu allocation failure: "
"page array size %lu allocation failed",
area->nr_pages * PAGE_SIZE, array_size);
+ free_vm_area(area);
return NULL;
}
--
2.30.2
