All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Jiri Bohac <jbohac@suse.cz>
Cc: cve@kernel.org, linux-kernel@vger.kernel.org,
	linux-cve-announce@vger.kernel.org,
	Eric Biederman <ebiederm@xmission.com>,
	kexec@lists.infradead.org
Subject: Re: CVE-2023-52823: kernel: kexec: copy user-array safely
Date: Fri, 24 May 2024 17:26:53 +0200	[thread overview]
Message-ID: <2024052440-irrigate-tightness-4a8a@gregkh> (raw)
In-Reply-To: <ZlCKLBjrF5PWt1hz@dwarf.suse.cz>

On Fri, May 24, 2024 at 02:38:04PM +0200, Jiri Bohac wrote:
> 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);
> 	}

Agreed, now rejected.

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

Yup, now rejected.

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

Also agreed, now rejected.

thanks,

greg k-h

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

WARNING: multiple messages have this Message-ID (diff)
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Jiri Bohac <jbohac@suse.cz>
Cc: cve@kernel.org, linux-kernel@vger.kernel.org,
	linux-cve-announce@vger.kernel.org,
	Eric Biederman <ebiederm@xmission.com>,
	kexec@lists.infradead.org
Subject: Re: CVE-2023-52823: kernel: kexec: copy user-array safely
Date: Fri, 24 May 2024 17:26:53 +0200	[thread overview]
Message-ID: <2024052440-irrigate-tightness-4a8a@gregkh> (raw)
In-Reply-To: <ZlCKLBjrF5PWt1hz@dwarf.suse.cz>

On Fri, May 24, 2024 at 02:38:04PM +0200, Jiri Bohac wrote:
> 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);
> 	}

Agreed, now rejected.

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

Yup, now rejected.

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

Also agreed, now rejected.

thanks,

greg k-h

  parent reply	other threads:[~2024-05-24 15:27 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-21 15:31 CVE-2023-52823: kernel: kexec: copy user-array safely Greg Kroah-Hartman
2024-05-24 10:02 ` Jiri Bohac
2024-05-24 10:02   ` Jiri Bohac
2024-05-24 10:15   ` Greg Kroah-Hartman
2024-05-24 10:15     ` Greg Kroah-Hartman
2024-05-24 12:38     ` Jiri Bohac
2024-05-24 12:38       ` Jiri Bohac
2024-05-24 14:13       ` Jiri Bohac
2024-05-24 14:13         ` Jiri Bohac
2024-05-24 15:27         ` Greg Kroah-Hartman
2024-05-24 15:27           ` Greg Kroah-Hartman
2024-05-24 15:26       ` Greg Kroah-Hartman [this message]
2024-05-24 15:26         ` Greg Kroah-Hartman

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=2024052440-irrigate-tightness-4a8a@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=cve@kernel.org \
    --cc=ebiederm@xmission.com \
    --cc=jbohac@suse.cz \
    --cc=kexec@lists.infradead.org \
    --cc=linux-cve-announce@vger.kernel.org \
    --cc=linux-kernel@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.