* (no subject)
@ 2018-01-22 6:55 Balasubramani Vivekanandan
2018-01-22 6:55 ` [PATCH] mm/slub.c: Fix wrong address during slab padding restoration Balasubramani Vivekanandan
0 siblings, 1 reply; 5+ messages in thread
From: Balasubramani Vivekanandan @ 2018-01-22 6:55 UTC (permalink / raw)
To: linux-mm; +Cc: balasubramani_vivekanandan
The start address sent to restore_bytes function is wrong. It points to
an location above the padding section. This is fixed in the attached patch
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] mm/slub.c: Fix wrong address during slab padding restoration
2018-01-22 6:55 Balasubramani Vivekanandan
@ 2018-01-22 6:55 ` Balasubramani Vivekanandan
0 siblings, 0 replies; 5+ messages in thread
From: Balasubramani Vivekanandan @ 2018-01-22 6:55 UTC (permalink / raw)
To: linux-mm; +Cc: balasubramani_vivekanandan
From: Balasubramani Vivekanandan <balasubramani_vivekanandan@mentor.com>
Start address calculated for slab padding restoration was wrong.
Wrong address would point to some section before padding and
could cause corruption
Signed-off-by: Balasubramani Vivekanandan <balasubramani_vivekanandan@mentor.com>
---
mm/slub.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index cfd56e5..733ba32 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -838,6 +838,7 @@ static int slab_pad_check(struct kmem_cache *s, struct page *page)
u8 *start;
u8 *fault;
u8 *end;
+ u8 *pad;
int length;
int remainder;
@@ -851,8 +852,9 @@ static int slab_pad_check(struct kmem_cache *s, struct page *page)
if (!remainder)
return 1;
+ pad = end - remainder;
metadata_access_enable();
- fault = memchr_inv(end - remainder, POISON_INUSE, remainder);
+ fault = memchr_inv(pad, POISON_INUSE, remainder);
metadata_access_disable();
if (!fault)
return 1;
@@ -860,9 +862,9 @@ static int slab_pad_check(struct kmem_cache *s, struct page *page)
end--;
slab_err(s, page, "Padding overwritten. 0x%p-0x%p", fault, end - 1);
- print_section(KERN_ERR, "Padding ", end - remainder, remainder);
+ print_section(KERN_ERR, "Padding ", pad, remainder);
- restore_bytes(s, "slab padding", POISON_INUSE, end - remainder, end);
+ restore_bytes(s, "slab padding", POISON_INUSE, fault, end);
return 0;
}
--
2.7.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/slub.c: Fix wrong address during slab padding restoration
2018-01-23 22:50 ` Andrew Morton
@ 2018-01-24 17:52 ` Balasubramani Vivekanandan
0 siblings, 0 replies; 5+ messages in thread
From: Balasubramani Vivekanandan @ 2018-01-24 17:52 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-mm
On Tue, Jan 23, 2018 at 02:50:26PM -0800, Andrew Morton wrote:
> On Mon, 22 Jan 2018 12:32:58 +0530 Balasubramani Vivekanandan <balasubramani_vivekanandan@mentor.com> wrote:
>
> > From: Balasubramani Vivekanandan <balasubramani_vivekanandan@mentor.com>
> >
> > Start address calculated for slab padding restoration was wrong.
> > Wrong address would point to some section before padding and
> > could cause corruption
> >
> > ...
> >
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -838,6 +838,7 @@ static int slab_pad_check(struct kmem_cache *s, struct page *page)
> > u8 *start;
> > u8 *fault;
> > u8 *end;
> > + u8 *pad;
> > int length;
> > int remainder;
> >
> > @@ -851,8 +852,9 @@ static int slab_pad_check(struct kmem_cache *s, struct page *page)
> > if (!remainder)
> > return 1;
> >
> > + pad = end - remainder;
> > metadata_access_enable();
> > - fault = memchr_inv(end - remainder, POISON_INUSE, remainder);
> > + fault = memchr_inv(pad, POISON_INUSE, remainder);
> > metadata_access_disable();
> > if (!fault)
> > return 1;
> > @@ -860,9 +862,9 @@ static int slab_pad_check(struct kmem_cache *s, struct page *page)
> > end--;
> >
> > slab_err(s, page, "Padding overwritten. 0x%p-0x%p", fault, end - 1);
> > - print_section(KERN_ERR, "Padding ", end - remainder, remainder);
> > + print_section(KERN_ERR, "Padding ", pad, remainder);
> >
> > - restore_bytes(s, "slab padding", POISON_INUSE, end - remainder, end);
> > + restore_bytes(s, "slab padding", POISON_INUSE, fault, end);
> > return 0;
> > }
>
> I don't see why it matters? The current code will overwrite
> POISON_INUSE bytes with POISON_INUSE, won't it?
>
> That's a bit strange but not incorrect?
Not really. The bug will overwrite into the object area with
POISON_INUSE.
The end pointer initially points to end of the padding area. Then
in the loop, end is decremented till it points to the end of the fault
area.
while (end > fault && end[-1] == POISON_INUSE)
end--;
Now using end - remainder, will not point to the begining of the padding
area but will sneak into the object area. So restore_bytes will
overwrite the object area
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/slub.c: Fix wrong address during slab padding restoration
2018-01-22 7:02 Balasubramani Vivekanandan
@ 2018-01-23 22:50 ` Andrew Morton
2018-01-24 17:52 ` Balasubramani Vivekanandan
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2018-01-23 22:50 UTC (permalink / raw)
To: Balasubramani Vivekanandan; +Cc: linux-mm
On Mon, 22 Jan 2018 12:32:58 +0530 Balasubramani Vivekanandan <balasubramani_vivekanandan@mentor.com> wrote:
> From: Balasubramani Vivekanandan <balasubramani_vivekanandan@mentor.com>
>
> Start address calculated for slab padding restoration was wrong.
> Wrong address would point to some section before padding and
> could cause corruption
>
> ...
>
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -838,6 +838,7 @@ static int slab_pad_check(struct kmem_cache *s, struct page *page)
> u8 *start;
> u8 *fault;
> u8 *end;
> + u8 *pad;
> int length;
> int remainder;
>
> @@ -851,8 +852,9 @@ static int slab_pad_check(struct kmem_cache *s, struct page *page)
> if (!remainder)
> return 1;
>
> + pad = end - remainder;
> metadata_access_enable();
> - fault = memchr_inv(end - remainder, POISON_INUSE, remainder);
> + fault = memchr_inv(pad, POISON_INUSE, remainder);
> metadata_access_disable();
> if (!fault)
> return 1;
> @@ -860,9 +862,9 @@ static int slab_pad_check(struct kmem_cache *s, struct page *page)
> end--;
>
> slab_err(s, page, "Padding overwritten. 0x%p-0x%p", fault, end - 1);
> - print_section(KERN_ERR, "Padding ", end - remainder, remainder);
> + print_section(KERN_ERR, "Padding ", pad, remainder);
>
> - restore_bytes(s, "slab padding", POISON_INUSE, end - remainder, end);
> + restore_bytes(s, "slab padding", POISON_INUSE, fault, end);
> return 0;
> }
I don't see why it matters? The current code will overwrite
POISON_INUSE bytes with POISON_INUSE, won't it?
That's a bit strange but not incorrect?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] mm/slub.c: Fix wrong address during slab padding restoration
@ 2018-01-22 7:02 Balasubramani Vivekanandan
2018-01-23 22:50 ` Andrew Morton
0 siblings, 1 reply; 5+ messages in thread
From: Balasubramani Vivekanandan @ 2018-01-22 7:02 UTC (permalink / raw)
To: linux-mm; +Cc: balasubramani_vivekanandan
From: Balasubramani Vivekanandan <balasubramani_vivekanandan@mentor.com>
Start address calculated for slab padding restoration was wrong.
Wrong address would point to some section before padding and
could cause corruption
Signed-off-by: Balasubramani Vivekanandan <balasubramani_vivekanandan@mentor.com>
---
mm/slub.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/mm/slub.c b/mm/slub.c
index cfd56e5..733ba32 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -838,6 +838,7 @@ static int slab_pad_check(struct kmem_cache *s, struct page *page)
u8 *start;
u8 *fault;
u8 *end;
+ u8 *pad;
int length;
int remainder;
@@ -851,8 +852,9 @@ static int slab_pad_check(struct kmem_cache *s, struct page *page)
if (!remainder)
return 1;
+ pad = end - remainder;
metadata_access_enable();
- fault = memchr_inv(end - remainder, POISON_INUSE, remainder);
+ fault = memchr_inv(pad, POISON_INUSE, remainder);
metadata_access_disable();
if (!fault)
return 1;
@@ -860,9 +862,9 @@ static int slab_pad_check(struct kmem_cache *s, struct page *page)
end--;
slab_err(s, page, "Padding overwritten. 0x%p-0x%p", fault, end - 1);
- print_section(KERN_ERR, "Padding ", end - remainder, remainder);
+ print_section(KERN_ERR, "Padding ", pad, remainder);
- restore_bytes(s, "slab padding", POISON_INUSE, end - remainder, end);
+ restore_bytes(s, "slab padding", POISON_INUSE, fault, end);
return 0;
}
--
2.7.4
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-01-24 17:52 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-22 6:55 Balasubramani Vivekanandan
2018-01-22 6:55 ` [PATCH] mm/slub.c: Fix wrong address during slab padding restoration Balasubramani Vivekanandan
2018-01-22 7:02 Balasubramani Vivekanandan
2018-01-23 22:50 ` Andrew Morton
2018-01-24 17:52 ` Balasubramani Vivekanandan
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).