On Thu, Mar 01, 2018 at 07:58:07PM +0530, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 3/1/2018 6:54 PM, Ville Syrjälä wrote:
> > On Thu, Mar 01, 2018 at 06:43:21PM +0530, Sharma, Shashank wrote:
> >> Regards
> >>
> >> Shashank
> >>
> >>
> >> On 2/24/2018 12:55 AM, Ville Syrjala wrote:
> >>> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >>>
> >>> While we want to potentially support multiple different gamma/degamma
> >>> LUT sizes we can (and should) at least check that the blob length
> >>> is a multiple of the LUT entry size.
> >> I dint understand the exact idea behind doing this, how is this going to
> >> benefit ? May be a bit more description ?
> > The benefit is rejecting garbage fed in from userspace.
> >
> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >>> ---
> >>> drivers/gpu/drm/drm_atomic.c | 15 +++++++++++----
> >>> 1 file changed, 11 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> >>> index 8945357212ba..933edec0299d 100644
> >>> --- a/drivers/gpu/drm/drm_atomic.c
> >>> +++ b/drivers/gpu/drm/drm_atomic.c
> >>> @@ -413,6 +413,7 @@ drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
> >>> struct drm_property_blob **blob,
> >>> uint64_t blob_id,
> >>> ssize_t expected_size,
> >>> + ssize_t expected_size_mod,
> >>> bool *replaced)
> >>> {
> >>> struct drm_property_blob *new_blob = NULL;
> >>> @@ -422,7 +423,13 @@ drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
> >>> if (new_blob == NULL)
> >>> return -EINVAL;
> >>>
> >>> - if (expected_size > 0 && expected_size != new_blob->length) {
> >>> + if (expected_size > 0 &&
> >>> + new_blob->length != expected_size) {
> >>> + drm_property_blob_put(new_blob);
> >>> + return -EINVAL;
> >>> + }
> >> One line needed here, matching the previous if() pattern
> > What line? Don't understand.
> I mean, I can see a blank line before previous if() condition, so lets
> keep the same pattern for this if() too
OTOH the two ifs are related so maybe just keep them together? Doesn't
actually matter to me though.
>
> - Shashank
> >>> + if (expected_size_mod > 0 &&
> >>> + new_blob->length % expected_size_mod != 0) {
> >>> drm_property_blob_put(new_blob);
> >>> return -EINVAL;
> >>> }
> >>> @@ -470,7 +477,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> >>> ret = drm_atomic_replace_property_blob_from_id(dev,
> >>> &state->degamma_lut,
> >>> val,
> >>> - -1,
> >>> + -1, sizeof(struct drm_color_lut),
> >>> &replaced);
> >>> state->color_mgmt_changed |= replaced;
> >>> return ret;
> >>> @@ -478,7 +485,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> >>> ret = drm_atomic_replace_property_blob_from_id(dev,
> >>> &state->ctm,
> >>> val,
> >>> - sizeof(struct drm_color_ctm),
> >>> + sizeof(struct drm_color_ctm), -1,
> >>> &replaced);
> >>> state->color_mgmt_changed |= replaced;
> >>> return ret;
> >>> @@ -486,7 +493,7 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> >>> ret = drm_atomic_replace_property_blob_from_id(dev,
> >>> &state->gamma_lut,
> >>> val,
> >>> - -1,
> >>> + -1, sizeof(struct drm_color_lut),
> >>> &replaced);
> >>> state->color_mgmt_changed |= replaced;
> >>> return ret;
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
