* [PATCH] drm: udl: Properly check framebuffer mmap offsets @ 2018-03-21 15:45 Greg Kroah-Hartman 2018-03-22 6:59 ` Daniel Vetter 0 siblings, 1 reply; 6+ messages in thread From: Greg Kroah-Hartman @ 2018-03-21 15:45 UTC (permalink / raw) To: David Airlie Cc: Tom Lendacky, Daniel Vetter, Eyal Itkin, Krzysztof Kozlowski, Cihangir Akturk, dri-devel, Ingo Molnar The memmap options sent to the udl framebuffer driver were not being checked for all sets of possible crazy values. Fix this up by properly bounding the allowed values. Reported-by: Eyal Itkin <eyalit@checkpoint.com> Cc: stable <stable@vger.kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c index b5b335c9b2bb..2ebdc6d5a76e 100644 --- a/drivers/gpu/drm/udl/udl_fb.c +++ b/drivers/gpu/drm/udl/udl_fb.c @@ -159,10 +159,15 @@ static int udl_fb_mmap(struct fb_info *info, struct vm_area_struct *vma) { unsigned long start = vma->vm_start; unsigned long size = vma->vm_end - vma->vm_start; - unsigned long offset = vma->vm_pgoff << PAGE_SHIFT; + unsigned long offset; unsigned long page, pos; - if (offset + size > info->fix.smem_len) + if (vma->vm_pgoff > (~0UL >> PAGE_SHIFT)) + return -EINVAL; + + offset = vma->vm_pgoff << PAGE_SHIFT; + + if (offset > info->fix.smem_len || size > info->fix.smem_len - offset) return -EINVAL; pos = (unsigned long)info->fix.smem_start + offset; _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: udl: Properly check framebuffer mmap offsets 2018-03-21 15:45 [PATCH] drm: udl: Properly check framebuffer mmap offsets Greg Kroah-Hartman @ 2018-03-22 6:59 ` Daniel Vetter 2018-03-22 8:03 ` Greg Kroah-Hartman 0 siblings, 1 reply; 6+ messages in thread From: Daniel Vetter @ 2018-03-22 6:59 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Tom Lendacky, David Airlie, Daniel Vetter, Eyal Itkin, dri-devel, Cihangir Akturk, Krzysztof Kozlowski, Ingo Molnar On Wed, Mar 21, 2018 at 04:45:53PM +0100, Greg Kroah-Hartman wrote: > The memmap options sent to the udl framebuffer driver were not being > checked for all sets of possible crazy values. Fix this up by properly > bounding the allowed values. > > Reported-by: Eyal Itkin <eyalit@checkpoint.com> > Cc: stable <stable@vger.kernel.org> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Applied to drm-misc-fixes, thanks for the patch. Does anyone working on overflow-proof integers? That would make a lot of this code so much simpler if we could just ask the compiler to carry the oferflow bit around for a given expression and then check that and bail with -EINVAL. -Daniel > > diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c > index b5b335c9b2bb..2ebdc6d5a76e 100644 > --- a/drivers/gpu/drm/udl/udl_fb.c > +++ b/drivers/gpu/drm/udl/udl_fb.c > @@ -159,10 +159,15 @@ static int udl_fb_mmap(struct fb_info *info, struct vm_area_struct *vma) > { > unsigned long start = vma->vm_start; > unsigned long size = vma->vm_end - vma->vm_start; > - unsigned long offset = vma->vm_pgoff << PAGE_SHIFT; > + unsigned long offset; > unsigned long page, pos; > > - if (offset + size > info->fix.smem_len) > + if (vma->vm_pgoff > (~0UL >> PAGE_SHIFT)) > + return -EINVAL; > + > + offset = vma->vm_pgoff << PAGE_SHIFT; > + > + if (offset > info->fix.smem_len || size > info->fix.smem_len - offset) > return -EINVAL; > > pos = (unsigned long)info->fix.smem_start + offset; -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: udl: Properly check framebuffer mmap offsets 2018-03-22 6:59 ` Daniel Vetter @ 2018-03-22 8:03 ` Greg Kroah-Hartman 2018-03-22 9:54 ` Daniel Vetter 0 siblings, 1 reply; 6+ messages in thread From: Greg Kroah-Hartman @ 2018-03-22 8:03 UTC (permalink / raw) To: Daniel Vetter Cc: Tom Lendacky, David Airlie, Daniel Vetter, Eyal Itkin, dri-devel, Cihangir Akturk, Krzysztof Kozlowski, Ingo Molnar On Thu, Mar 22, 2018 at 07:59:59AM +0100, Daniel Vetter wrote: > Does anyone working on overflow-proof integers? That would make a lot of > this code so much simpler if we could just ask the compiler to carry the > oferflow bit around for a given expression and then check that and bail > with -EINVAL. That would be nice, but no, I don't think that's part of any C standard work that I have heard of :( greg k-h _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: udl: Properly check framebuffer mmap offsets 2018-03-22 8:03 ` Greg Kroah-Hartman @ 2018-03-22 9:54 ` Daniel Vetter 2018-03-23 0:14 ` Kees Cook 0 siblings, 1 reply; 6+ messages in thread From: Daniel Vetter @ 2018-03-22 9:54 UTC (permalink / raw) To: Greg Kroah-Hartman, Kees Cook, kernel-hardening Cc: Tom Lendacky, David Airlie, Eyal Itkin, Krzysztof Kozlowski, Cihangir Akturk, dri-devel, Ingo Molnar On Thu, Mar 22, 2018 at 9:03 AM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Thu, Mar 22, 2018 at 07:59:59AM +0100, Daniel Vetter wrote: >> Does anyone working on overflow-proof integers? That would make a lot of >> this code so much simpler if we could just ask the compiler to carry the >> oferflow bit around for a given expression and then check that and bail >> with -EINVAL. > > That would be nice, but no, I don't think that's part of any C standard > work that I have heard of :( Well we have refcount_t already, stitching something together that would work and not suck too badly with performance should be possible. But yeah direct compiler support would be better (and would allow optimizing the carry flag checks I guess). I kinda hoped Kees&team would be working on this eventually. Adding Kees+kernel-hardening, maybe he'll grow fond of this :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: udl: Properly check framebuffer mmap offsets 2018-03-22 9:54 ` Daniel Vetter @ 2018-03-23 0:14 ` Kees Cook 2018-03-26 8:26 ` Daniel Vetter 0 siblings, 1 reply; 6+ messages in thread From: Kees Cook @ 2018-03-23 0:14 UTC (permalink / raw) To: Daniel Vetter Cc: Greg Kroah-Hartman, Kernel Hardening, David Airlie, dri-devel, Sean Paul, Cihangir Akturk, Ingo Molnar, Krzysztof Kozlowski, Tom Lendacky, Eyal Itkin, Daniel Micay On Thu, Mar 22, 2018 at 2:54 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Thu, Mar 22, 2018 at 9:03 AM, Greg Kroah-Hartman > <gregkh@linuxfoundation.org> wrote: >> On Thu, Mar 22, 2018 at 07:59:59AM +0100, Daniel Vetter wrote: >>> Does anyone working on overflow-proof integers? That would make a lot of >>> this code so much simpler if we could just ask the compiler to carry the >>> oferflow bit around for a given expression and then check that and bail >>> with -EINVAL. >> >> That would be nice, but no, I don't think that's part of any C standard >> work that I have heard of :( > > Well we have refcount_t already, stitching something together that > would work and not suck too badly with performance should be possible. > But yeah direct compiler support would be better (and would allow > optimizing the carry flag checks I guess). I kinda hoped Kees&team > would be working on this eventually. refcount_t could be used if it happens to match the needed semantics. > Adding Kees+kernel-hardening, maybe he'll grow fond of this :-) Yeah, general integer overflow is on the list of things to get fixed in the kernel. It's a bit of a long road, though. Clang has -fsanitize=integer (and sub-options) which could be added for specific object or trees: https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#ubsan-checks GCC seems to only support manual marking of overflow detections: https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html grsecurity/PaX has a gcc plugin for overflow detection, though it hasn't been upstreamed and comes with various caveats: http://forums.grsecurity.net/viewtopic.php?f=7&t=3043 https://github.com/ephox-gcc-plugins/size_overflow -Kees -- Kees Cook Pixel Security ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm: udl: Properly check framebuffer mmap offsets 2018-03-23 0:14 ` Kees Cook @ 2018-03-26 8:26 ` Daniel Vetter 0 siblings, 0 replies; 6+ messages in thread From: Daniel Vetter @ 2018-03-26 8:26 UTC (permalink / raw) To: Kees Cook Cc: Tom Lendacky, Kernel Hardening, David Airlie, Greg Kroah-Hartman, Eyal Itkin, dri-devel, Cihangir Akturk, Daniel Micay, Krzysztof Kozlowski, Ingo Molnar On Thu, Mar 22, 2018 at 05:14:40PM -0700, Kees Cook wrote: > On Thu, Mar 22, 2018 at 2:54 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > > On Thu, Mar 22, 2018 at 9:03 AM, Greg Kroah-Hartman > > <gregkh@linuxfoundation.org> wrote: > >> On Thu, Mar 22, 2018 at 07:59:59AM +0100, Daniel Vetter wrote: > >>> Does anyone working on overflow-proof integers? That would make a lot of > >>> this code so much simpler if we could just ask the compiler to carry the > >>> oferflow bit around for a given expression and then check that and bail > >>> with -EINVAL. > >> > >> That would be nice, but no, I don't think that's part of any C standard > >> work that I have heard of :( > > > > Well we have refcount_t already, stitching something together that > > would work and not suck too badly with performance should be possible. > > But yeah direct compiler support would be better (and would allow > > optimizing the carry flag checks I guess). I kinda hoped Kees&team > > would be working on this eventually. > > refcount_t could be used if it happens to match the needed semantics. > > > Adding Kees+kernel-hardening, maybe he'll grow fond of this :-) > > Yeah, general integer overflow is on the list of things to get fixed > in the kernel. It's a bit of a long road, though. > > Clang has -fsanitize=integer (and sub-options) which could be added > for specific object or trees: > https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#ubsan-checks I expect that a bunch of folks will scream about checking all interger math for overflows > GCC seems to only support manual marking of overflow detections: > https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html And this sounds _very_ verbose. What I have in mind is something like result = overflow_checked(expr, &overflow) if (overflow) return -EINVAL; and expr could be any expression at all (except calling functions ofc, or whatever the correct verbiage from the C standard for this is). I don't want to annotate each compuation individually at least, that's too much typing :-) Even neater would be an entire block: while { /* compute stuff except function calls, but allow inlining */ } has_overflow { return -EINVAL; } Anyway, yay for this being somewhere on the future plans! > grsecurity/PaX has a gcc plugin for overflow detection, though it > hasn't been upstreamed and comes with various caveats: > http://forums.grsecurity.net/viewtopic.php?f=7&t=3043 > https://github.com/ephox-gcc-plugins/size_overflow Yeah this looks fairly fragile to me, also can't use the overflow bits instructions sets have I think. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-03-26 8:26 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-03-21 15:45 [PATCH] drm: udl: Properly check framebuffer mmap offsets Greg Kroah-Hartman 2018-03-22 6:59 ` Daniel Vetter 2018-03-22 8:03 ` Greg Kroah-Hartman 2018-03-22 9:54 ` Daniel Vetter 2018-03-23 0:14 ` Kees Cook 2018-03-26 8:26 ` Daniel Vetter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox