Date: Tue, 30 May 2017 19:47:27 +0000 From: bugzilla-noreply@freebsd.org To: freebsd-bugs@FreeBSD.org Subject: [Bug 219525] Multiple bugs in mpr ioctl handler Message-ID: <bug-219525-8-PZzKg4k89F@https.bugs.freebsd.org/bugzilla/> In-Reply-To: <bug-219525-8@https.bugs.freebsd.org/bugzilla/> References: <bug-219525-8@https.bugs.freebsd.org/bugzilla/>
next in thread | previous in thread | raw e-mail | index | archive | help
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=219525 --- Comment #1 from Stephen McConnell <slm@freebsd.org> --- Thanks for the report. I know there are a few issues there, especially with the driver not checking for bad user supplied values. I agree with most of your comments, but I have a question. In this section: ..... mpr_lock(sc); size = data->Size; if ((size >= sizeof(sc->recorded_events)) && (status == 0)) { mpr_unlock(sc); if (copyout((void *)sc->recorded_events, PTRIN(data->PtrEvents), size) != 0) status = EFAULT; mpr_lock(sc); } else { /* * data->Size value is not large enough to copy event data. */ status = EFAULT; } /* * Change size value to match the number of bytes that were copied. */ if (status == 0) data->Size = sizeof(sc->recorded_events); mpr_unlock(sc); ..... You say: "It checks if the user supplied size is greater than the the size of the struct, and if so, copies using the size which it just determined was too large. It should `copyout` using `sizeof(sc->recorded_events)`." But this is just checking if the user supplied size is enough to hold sizeof(sc->recored_events). It's not checking if it was too large, it's checking if it's large enough. If that user supplied size is >= to the data being copied, it should be OK. Do you agree? I'm not sure I like that data->Size is being changed from the user supplied value, but that's something different. Maybe that's OK, but not sure. It could be that this is a normal technique used to return the number of bytes that are valid in the user buffer, but I only see that being done in this one function. -- You are receiving this mail because: You are the assignee for the bug.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bug-219525-8-PZzKg4k89F>