On Fri, May 24, 2024 at 12:15:47PM +0200, Greg Kroah-Hartman wrote:
> Nice, but then why was this commit worded this way?  Now we check twice?
> Double safe?  Should it be reverted?

double safe's good; turning it into a CVE not so much :(
CVE-2023-52822, CVE-2023-52824 and CVE-2023-52820, originally from the same 
patch
series, seem to be the exact same case.

CVE-2023-52822:

        int vmw_surface_define_ioctl(struct drm_device *dev, void *data,
                                     struct drm_file *file_priv)
        {
        ...
                if (num_sizes > DRM_VMW_MAX_SURFACE_FACES * 
DRM_VMW_MAX_MIP_LEVELS ||
                    num_sizes == 0)
                        return -EINVAL;
        ...
                metadata->num_sizes = num_sizes;
                metadata->sizes =
                        memdup_user((struct drm_vmw_size __user *)(unsigned 
long)
                                    req->size_addr,
                                    sizeof(*metadata->sizes) * 
metadata->num_sizes);
        }

CVE-2023-52824 (here the check is in the immediately preceeding statement, 
could it
be any more obvious?):

        long watch_queue_set_filter(struct pipe_inode_info *pipe,
                                    struct watch_notification_filter __user 
*_filter)
        {
                if (filter.nr_filters == 0 ||
                    filter.nr_filters > 16 ||
                    filter.__reserved != 0)
                        return -EINVAL;

                tf = memdup_user(_filter->filters, filter.nr_filters * 
sizeof(*tf));
        }


CVE-2023-52820 is a little less obvious to be safe, but I believe it is:

        int drm_mode_create_lease_ioctl(struct drm_device *dev,
                                        void *data, struct drm_file 
*lessor_priv)
        {
        ...
                        object_ids = 
memdup_user(u64_to_user_ptr(cl->object_ids),
                                                 array_size(object_count, 
sizeof(__u32)));

        array_size() will safely multiply object_count * 4 and return SIZE_MAX 
on
        overflow, making the kmalloc inside memdup_user cleanly fail with 
-ENOMEM.


> I'll go revoke this, thanks for the review!

could you check and revoke all the above as well?

Thanks,

-- 
Jiri Bohac <jbo...@suse.cz>
SUSE Labs, Prague, Czechia


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Reply via email to