Re: [PATCH 3/6] drm: Verify gamma/degamma LUT size

["Sharma, Shashank" <shashank.sharma@intel.com>]

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@linux.intel.com>
>>>
>>> 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@linux.intel.com>
>>> ---
>>>    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

- 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;

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx