On Wed, Oct 2, 2019 at 7:27 AM Jason Gunthorpe <[email protected]> wrote: > > On Tue, Oct 01, 2019 at 11:32:50AM -0700, Alan Mikhak wrote: > > From: Alan Mikhak <[email protected]> > > > > Update the description of sg_set_page() to communicate current > > requirements for the page pointer parameter. > > > > Signed-off-by: Alan Mikhak <[email protected]> > > include/linux/scatterlist.h | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h > > index 6eec50fb36c8..6dda865893aa 100644 > > +++ b/include/linux/scatterlist.h > > @@ -112,6 +112,12 @@ static inline void sg_assign_page(struct scatterlist > > *sg, struct page *page) > > * of the page pointer. See sg_page() for looking up the page belonging > > * to an sg entry. > > * > > + * Scatterlist currently expects the page parameter to be a pointer to > > + * a page that is backed by a page struct. > > + * > > + * Page pointers derived from addresses obtained from ioremap() are > > + * currently not supported since they require use of iomem safe memcpy. > > + * > > **/ > > static inline void sg_set_page(struct scatterlist *sg, struct page *page, > > unsigned int len, unsigned int offset) > > It seems a bit weird to have a comment explaining that 'struct page > *page' must actually be a valid pointer. Of course it must. > > Computing a 'struct page *' to something that doesn't actually have a > struct page is simply a bug in whoever did that. > > Code should never be interchanging ioremap results with the struct > page* world. > > Jason
Agreed. Should scatterlist API sg_set_page() check the page pointer to prevent such usage that could crash the system? Is it still reasonable to at least tell the programmer what is not allowed by adding a comment in the description? Alan
