All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: DRI Development <dri-devel@lists.freedesktop.org>,
	LKML <linux-kernel@vger.kernel.org>,
	kvm@vger.kernel.org, linux-mm@kvack.org,
	linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc@vger.kernel.org, linux-media@vger.kernel.org,
	linux-s390@vger.kernel.org,
	Daniel Vetter <daniel.vetter@intel.com>,
	Jason Gunthorpe <jgg@ziepe.ca>, Kees Cook <keescook@chromium.org>,
	Dan Williams <dan.j.williams@intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	John Hubbard <jhubbard@nvidia.com>,
	J??r??me Glisse <jglisse@redhat.com>, Jan Kara <jack@suse.cz>
Subject: Re: [PATCH v4 08/15] mm: Add unsafe_follow_pfn
Date: Thu, 29 Oct 2020 08:56:44 +0000	[thread overview]
Message-ID: <20201029085644.GA25658@infradead.org> (raw)
In-Reply-To: <20201026105818.2585306-9-daniel.vetter@ffwll.ch>

> +int unsafe_follow_pfn(struct vm_area_struct *vma, unsigned long address,
> +	unsigned long *pfn)

The one tab indent here looks weird, normally tis would be two tabs
or aligned aftetthe opening brace.

> +{
> +#ifdef CONFIG_STRICT_FOLLOW_PFN
> +	pr_info("unsafe follow_pfn usage rejected, see CONFIG_STRICT_FOLLOW_PFN\n");
> +	return -EINVAL;
> +#else
> +	WARN_ONCE(1, "unsafe follow_pfn usage\n");
> +	add_taint(TAINT_USER, LOCKDEP_STILL_OK);
> +
> +	return follow_pfn(vma, address, pfn);
> +#endif

Woudn't this be a pretty good use case of "if (IS_ENABLED(...)))"?

Also I'd expect the inverse polarity of the config option, that is
a USAFE_FOLLOW_PFN option to enable to unsafe behavior.

> +/**
> + * unsafe_follow_pfn - look up PFN at a user virtual address
> + * @vma: memory mapping
> + * @address: user virtual address
> + * @pfn: location to store found PFN
> + *
> + * Only IO mappings and raw PFN mappings are allowed.
> + *
> + * Returns zero and the pfn at @pfn on success, -ve otherwise.
> + */
> +int unsafe_follow_pfn(struct vm_area_struct *vma, unsigned long address,
> +	unsigned long *pfn)
> +{
> +	return follow_pfn(vma, address, pfn);
> +}
> +EXPORT_SYMBOL(unsafe_follow_pfn);

Any reason this doesn't use the warn and disable logic?

WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@infradead.org>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: linux-s390@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
	Jan Kara <jack@suse.cz>, Kees Cook <keescook@chromium.org>,
	kvm@vger.kernel.org, Jason Gunthorpe <jgg@ziepe.ca>,
	John Hubbard <jhubbard@nvidia.com>,
	LKML <linux-kernel@vger.kernel.org>,
	DRI Development <dri-devel@lists.freedesktop.org>,
	linux-mm@kvack.org, J??r??me Glisse <jglisse@redhat.com>,
	Daniel Vetter <daniel.vetter@intel.com>,
	Dan Williams <dan.j.williams@intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-media@vger.kernel.org
Subject: Re: [PATCH v4 08/15] mm: Add unsafe_follow_pfn
Date: Thu, 29 Oct 2020 08:56:44 +0000	[thread overview]
Message-ID: <20201029085644.GA25658@infradead.org> (raw)
In-Reply-To: <20201026105818.2585306-9-daniel.vetter@ffwll.ch>

> +int unsafe_follow_pfn(struct vm_area_struct *vma, unsigned long address,
> +	unsigned long *pfn)

The one tab indent here looks weird, normally tis would be two tabs
or aligned aftetthe opening brace.

> +{
> +#ifdef CONFIG_STRICT_FOLLOW_PFN
> +	pr_info("unsafe follow_pfn usage rejected, see CONFIG_STRICT_FOLLOW_PFN\n");
> +	return -EINVAL;
> +#else
> +	WARN_ONCE(1, "unsafe follow_pfn usage\n");
> +	add_taint(TAINT_USER, LOCKDEP_STILL_OK);
> +
> +	return follow_pfn(vma, address, pfn);
> +#endif

Woudn't this be a pretty good use case of "if (IS_ENABLED(...)))"?

Also I'd expect the inverse polarity of the config option, that is
a USAFE_FOLLOW_PFN option to enable to unsafe behavior.

> +/**
> + * unsafe_follow_pfn - look up PFN at a user virtual address
> + * @vma: memory mapping
> + * @address: user virtual address
> + * @pfn: location to store found PFN
> + *
> + * Only IO mappings and raw PFN mappings are allowed.
> + *
> + * Returns zero and the pfn at @pfn on success, -ve otherwise.
> + */
> +int unsafe_follow_pfn(struct vm_area_struct *vma, unsigned long address,
> +	unsigned long *pfn)
> +{
> +	return follow_pfn(vma, address, pfn);
> +}
> +EXPORT_SYMBOL(unsafe_follow_pfn);

Any reason this doesn't use the warn and disable logic?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-10-29  9:00 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-26 10:58 [PATCH v4 00/15] follow_pfn and other iomap races Daniel Vetter
2020-10-26 10:58 ` Daniel Vetter
2020-10-26 10:58 ` Daniel Vetter
2020-10-26 10:58 ` [PATCH v4 01/15] drm/exynos: Stop using frame_vector helpers Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58 ` [PATCH v4 02/15] drm/exynos: Use FOLL_LONGTERM for g2d cmdlists Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58 ` [PATCH v4 03/15] misc/habana: Stop using frame_vector helpers Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58 ` [PATCH v4 04/15] misc/habana: Use FOLL_LONGTERM for userptr Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58 ` [PATCH v4 05/15] mm/frame-vector: Use FOLL_LONGTERM Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 22:15   ` Tomasz Figa
2020-10-26 22:15     ` Tomasz Figa
2020-10-26 22:15     ` Tomasz Figa
2020-10-26 22:15     ` Tomasz Figa
2020-10-27  8:05     ` Daniel Vetter
2020-10-27  8:05       ` Daniel Vetter
2020-10-27  8:05       ` Daniel Vetter
2020-10-26 10:58 ` [PATCH v4 06/15] media: videobuf2: Move frame_vector into media subsystem Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 22:03   ` Tomasz Figa
2020-10-26 22:03     ` Tomasz Figa
2020-10-26 22:03     ` Tomasz Figa
2020-10-26 22:03     ` Tomasz Figa
2020-10-26 10:58 ` [PATCH v4 07/15] mm: Close race in generic_access_phys Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58 ` [PATCH v4 08/15] mm: Add unsafe_follow_pfn Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-29  8:56   ` Christoph Hellwig [this message]
2020-10-29  8:56     ` Christoph Hellwig
2020-10-29  9:23     ` Daniel Vetter
2020-10-29  9:23       ` Daniel Vetter
2020-10-29  9:23       ` Daniel Vetter
2020-10-29  9:23       ` Daniel Vetter
2020-10-29  9:23       ` Daniel Vetter
2020-10-26 10:58 ` [PATCH v4 09/15] media/videbuf1|2: Mark follow_pfn usage as unsafe Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 22:02   ` Tomasz Figa
2020-10-26 22:02     ` Tomasz Figa
2020-10-26 22:02     ` Tomasz Figa
2020-10-26 22:02     ` Tomasz Figa
2020-10-26 10:58 ` [PATCH v4 10/15] vfio/type1: Mark follow_pfn " Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58 ` [PATCH v4 11/15] PCI: Obey iomem restrictions for procfs mmap Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58 ` [PATCH v4 12/15] /dev/mem: Only set filp->f_mapping Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58 ` [PATCH v4 13/15] resource: Move devmem revoke code to resource framework Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58 ` [PATCH v4 14/15] sysfs: Support zapping of binary attr mmaps Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58 ` [PATCH v4 15/15] PCI: Revoke mappings like devmem Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-26 10:58   ` Daniel Vetter
2020-10-29  8:57 ` [PATCH v4 00/15] follow_pfn and other iomap races Christoph Hellwig
2020-10-29  8:57   ` Christoph Hellwig
2020-10-29  9:25   ` Daniel Vetter
2020-10-29  9:25     ` Daniel Vetter
2020-10-29  9:25     ` Daniel Vetter
2020-10-29  9:25     ` Daniel Vetter
2020-10-29  9:25     ` Daniel Vetter
2020-10-29  9:28     ` Christoph Hellwig
2020-10-29  9:28       ` Christoph Hellwig
2020-10-29  9:28       ` Christoph Hellwig
2020-10-29  9:38       ` Daniel Vetter
2020-10-29  9:38         ` Daniel Vetter
2020-10-29  9:38         ` Daniel Vetter
2020-10-29  9:38         ` Daniel Vetter
2020-10-29  9:38         ` Daniel Vetter
2020-10-29 10:01         ` Christoph Hellwig
2020-10-29 10:01           ` Christoph Hellwig
2020-10-29 10:01           ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201029085644.GA25658@infradead.org \
    --to=hch@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=dan.j.williams@intel.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=daniel.vetter@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jack@suse.cz \
    --cc=jgg@ziepe.ca \
    --cc=jglisse@redhat.com \
    --cc=jhubbard@nvidia.com \
    --cc=keescook@chromium.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.