Add VM_BUG_ON() bounds checking to make sure that, if "offset + len>
PAGE_SIZE", memset() does not corrupt data in adjacent pages.
Cc: Ira Weiny <[email protected]>
Signed-off-by: Fabio M. De Francesco <[email protected]>
---
include/linux/highmem.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/highmem.h b/include/linux/highmem.h
index 6b2d59e025c5..d54dbaae9a5e 100644
--- a/include/linux/highmem.h
+++ b/include/linux/highmem.h
@@ -380,6 +380,8 @@ static inline void memcpy_to_page(struct page *page, size_t offset,
static inline void memzero_page(struct page *page, size_t offset, size_t len)
{
char *addr = kmap_local_page(page);
+
+ VM_BUG_ON(offset + len > PAGE_SIZE);
memset(addr + offset, 0, len);
flush_dcache_page(page);
kunmap_local(addr);
--
2.34.1
On martedì 26 aprile 2022 22:48:11 CEST Andrew Morton wrote:
> On Tue, 26 Apr 2022 22:19:57 +0200 "Fabio M. De Francesco"
<[email protected]> wrote:
>
> > On martedì 26 aprile 2022 21:34:12 CEST Andrew Morton wrote:
> > > On Tue, 26 Apr 2022 21:30:20 +0200 "Fabio M. De Francesco"
> > <[email protected]> wrote:
> > >
> > > > Add VM_BUG_ON() bounds checking to make sure that, if "offset +
len>
> > > > PAGE_SIZE", memset() does not corrupt data in adjacent pages.
> > > >
> > >
> > > hm, why? To match all the other functions in there?
> > >
> > > I suppose that's logical. Or we could just delete all the other
> > > VM_BUG_ON()s. Have any of them proven to be at all useful?
> > >
> > I am not so sure about it being so useful. I just noted that
memzero_page()
> > is the only function of that family that is implemented with no
> > VM_BUG_ON(). I have no actual proofs of usefulness :(
> >
> > This is why yesterday I sent an "RFC Patch" (please see
> > https://lore.kernel.org/lkml/[email protected]/
> >
> > Soon after sending it I thought that VM_WARN_ON_ONCE() could have been
> > better suited, but Ira Weiny wrote to use VM_BUG_ON() for consistency.
> >
> > Now I could either delete all other VM_BUG_ON() or replace them with
> > VM_WARN_ON_ONCE() (or some other macro).
> >
> > Ah, a third solution might be to leave highmem.h as it is now :)
> >
> > What do you prefer?
>
> Merge this patch as-is, I guess. Going through and removing unuseful
> VM_BUG_ON()s is a separable activity.
>
Thanks!
While at this, I've just noted that you sent a confirmation which lists all
the patches of mine that are currently in your tree.
I see that you took v1 of "Extend and reorganize Highmem's documentation".
I suppose that you missed the v2 of that series at
https://lore.kernel.org/lkml/[email protected]/
Can you please check it?
Thanks,
Fabio
On Tue, 26 Apr 2022 22:19:57 +0200 "Fabio M. De Francesco" <[email protected]> wrote:
> On marted? 26 aprile 2022 21:34:12 CEST Andrew Morton wrote:
> > On Tue, 26 Apr 2022 21:30:20 +0200 "Fabio M. De Francesco"
> <[email protected]> wrote:
> >
> > > Add VM_BUG_ON() bounds checking to make sure that, if "offset + len>
> > > PAGE_SIZE", memset() does not corrupt data in adjacent pages.
> > >
> >
> > hm, why? To match all the other functions in there?
> >
> > I suppose that's logical. Or we could just delete all the other
> > VM_BUG_ON()s. Have any of them proven to be at all useful?
> >
> I am not so sure about it being so useful. I just noted that memzero_page()
> is the only function of that family that is implemented with no
> VM_BUG_ON(). I have no actual proofs of usefulness :(
>
> This is why yesterday I sent an "RFC Patch" (please see
> https://lore.kernel.org/lkml/[email protected]/
>
> Soon after sending it I thought that VM_WARN_ON_ONCE() could have been
> better suited, but Ira Weiny wrote to use VM_BUG_ON() for consistency.
>
> Now I could either delete all other VM_BUG_ON() or replace them with
> VM_WARN_ON_ONCE() (or some other macro).
>
> Ah, a third solution might be to leave highmem.h as it is now :)
>
> What do you prefer?
Merge this patch as-is, I guess. Going through and removing unuseful
VM_BUG_ON()s is a separable activity.
On Tue, 26 Apr 2022 21:30:20 +0200 "Fabio M. De Francesco" <[email protected]> wrote:
> Add VM_BUG_ON() bounds checking to make sure that, if "offset + len>
> PAGE_SIZE", memset() does not corrupt data in adjacent pages.
>
hm, why? To match all the other functions in there?
I suppose that's logical. Or we could just delete all the other
VM_BUG_ON()s. Have any of them proven to be at all useful?
On martedì 26 aprile 2022 21:34:12 CEST Andrew Morton wrote:
> On Tue, 26 Apr 2022 21:30:20 +0200 "Fabio M. De Francesco"
<[email protected]> wrote:
>
> > Add VM_BUG_ON() bounds checking to make sure that, if "offset + len>
> > PAGE_SIZE", memset() does not corrupt data in adjacent pages.
> >
>
> hm, why? To match all the other functions in there?
>
> I suppose that's logical. Or we could just delete all the other
> VM_BUG_ON()s. Have any of them proven to be at all useful?
>
I am not so sure about it being so useful. I just noted that memzero_page()
is the only function of that family that is implemented with no
VM_BUG_ON(). I have no actual proofs of usefulness :(
This is why yesterday I sent an "RFC Patch" (please see
https://lore.kernel.org/lkml/[email protected]/
Soon after sending it I thought that VM_WARN_ON_ONCE() could have been
better suited, but Ira Weiny wrote to use VM_BUG_ON() for consistency.
Now I could either delete all other VM_BUG_ON() or replace them with
VM_WARN_ON_ONCE() (or some other macro).
Ah, a third solution might be to leave highmem.h as it is now :)
What do you prefer?
Thanks,
Fabio