On Tue, Oct 05, 2021 at 11:12:47PM +0100, Matthew Wilcox wrote:
> On Tue, Oct 05, 2021 at 02:26:37PM -0700, Kees Cook wrote:
> > On Mon, Oct 04, 2021 at 11:42:23PM +0100, Matthew Wilcox (Oracle) wrote:
> > > + } else if (PageHead(page)) {
> > > + /* A compound allocation */
> > > + if (ptr + n > page_address(page) + page_size(page))
> > > + usercopy_abort("page alloc", NULL, to_user, 0, n);
> >
> > "0" could be "ptr - page_address(page)", I think? With that:
> >
> > Acked-by: Kees Cook <keescook@xxxxxxxxxxxx>
>
> Right, so that can be:
>
> } else if (PageHead(page)) {
> /* A compound allocation */
> unsigned long offset = ptr - page_address(page);
> if (offset + n > page_size(page))
> usercopy_abort("page alloc", NULL, to_user, offset, n);
>
> which saves us calling page_address() twice. Probably GCC is smart
> enough to CSE it anyway, but it also avoids splitting at the 80 column
> boundary ;-)
Perfect, yes!
--
Kees Cook
