LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Brian Starkey <Brian.Starkey@arm.com>
Cc: "james qian wang (Arm Technology China)"
	<james.qian.wang@arm.com>, nd <nd@arm.com>,
	"Lowry Li (Arm Technology China)" <Lowry.Li@arm.com>,
	"Tiannan Zhu (Arm Technology China)" <Tiannan.Zhu@arm.com>,
	"airlied@linux.ie" <airlied@linux.ie>,
	Liviu Dudau <Liviu.Dudau@arm.com>,
	"Jonathan Chai (Arm Technology China)" <Jonathan.Chai@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"Julien Yin (Arm Technology China)" <Julien.Yin@arm.com>,
	"Yiqi Kang (Arm Technology China)" <Yiqi.Kang@arm.com>,
	"thomas Sun (Arm Technology China)" <thomas.Sun@arm.com>,
	Ayan Halder <Ayan.Halder@arm.com>,
	"sean@poorly.run" <sean@poorly.run>
Subject: Re: [PATCH] drm/komeda: Added AFBC support for komeda driver
Date: Fri, 24 May 2019 15:12:26 +0300	[thread overview]
Message-ID: <20190524121226.GD5942@intel.com> (raw)
In-Reply-To: <20190524111009.beddu67vvx66wvmk@DESKTOP-E1NTVVP.localdomain>

On Fri, May 24, 2019 at 11:10:09AM +0000, Brian Starkey wrote:
> Hi,
> 
> On Tue, May 21, 2019 at 09:45:58AM +0100, james qian wang (Arm Technology China) wrote:
> > On Thu, May 16, 2019 at 09:57:49PM +0800, Ayan Halder wrote:
> > > On Thu, Apr 04, 2019 at 12:06:14PM +0100, james qian wang (Arm Technology China) wrote:
> > > >  
> > > > +static int
> > > > +komeda_fb_afbc_size_check(struct komeda_fb *kfb, struct drm_file *file,
> > > > +			  const struct drm_mode_fb_cmd2 *mode_cmd)
> > > > +{
> > > > +	struct drm_framebuffer *fb = &kfb->base;
> > > > +	const struct drm_format_info *info = fb->format;
> > > > +	struct drm_gem_object *obj;
> > > > +	u32 alignment_w = 0, alignment_h = 0, alignment_header;
> > > > +	u32 n_blocks = 0, min_size = 0;
> > > > +
> > > > +	obj = drm_gem_object_lookup(file, mode_cmd->handles[0]);
> > > > +	if (!obj) {
> > > > +		DRM_DEBUG_KMS("Failed to lookup GEM object\n");
> > > > +		return -ENOENT;
> > > > +	}
> > > > +
> > > > +	switch (fb->modifier & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) {
> > > > +	case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8:
> > > > +		alignment_w = 32;
> > > > +		alignment_h = 8;
> > > > +		break;
> > > > +	case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16:
> > > > +		alignment_w = 16;
> > > > +		alignment_h = 16;
> > > > +		break;
> > > > +	default:
> > > Can we have something like a warn here ?
> > 
> > will add a WARN here.
> > 
> 
> I think it's better not to. fb->modifier comes from
> userspace, so a malicious app could spam us with WARNs, effectively
> dos-ing the system. -EINVAL should be sufficient.

Should probably check that the entire modifier+format is
actually valid. Otherwise you risk passing on a bogus
modifier deeper into the driver which may trigger
interesting bugs.

Also theoretically (however unlikely) some broken userspace
might start to depend on the ability to create framebuffers
with crap modifiers, which could later break if you change
the way you handle the modifiers. Then you're stuck between
the rock and hard place because you can't break existing
userspace but you still want to change the way modifiers
are handled in the kernel.

Best not give userspace too much rope IMO. Two ways to go about
that:
1) drm_any_plane_has_format() (assumes your .format_mod_supported()
   does its job properly)
2) roll your own 

-- 
Ville Syrjälä
Intel

  reply	other threads:[~2019-05-24 12:12 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-04 11:06 james qian wang (Arm Technology China)
2019-05-16 13:57 ` Ayan Halder
2019-05-21  8:45   ` james qian wang (Arm Technology China)
2019-05-24 11:10     ` Brian Starkey
2019-05-24 12:12       ` Ville Syrjälä [this message]
2019-05-27  6:51         ` james qian wang (Arm Technology China)
2019-05-27 13:19           ` Ville Syrjälä
2019-05-28 15:26           ` Brian Starkey

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=20190524121226.GD5942@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=Ayan.Halder@arm.com \
    --cc=Brian.Starkey@arm.com \
    --cc=Jonathan.Chai@arm.com \
    --cc=Julien.Yin@arm.com \
    --cc=Liviu.Dudau@arm.com \
    --cc=Lowry.Li@arm.com \
    --cc=Tiannan.Zhu@arm.com \
    --cc=Yiqi.Kang@arm.com \
    --cc=airlied@linux.ie \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=james.qian.wang@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nd@arm.com \
    --cc=sean@poorly.run \
    --cc=thomas.Sun@arm.com \
    --subject='Re: [PATCH] drm/komeda: Added AFBC support for komeda driver' \
    /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

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).