Bug 196119 - x11-server/xorg-server: Unbreak net/tigervnc after CVE-2014-8092 patch
Summary: x11-server/xorg-server: Unbreak net/tigervnc after CVE-2014-8092 patch
Status: Closed Overcome By Events
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-ports-bugs (Nobody)
URL:
Keywords: needs-qa, patch
Depends on:
Blocks:
 
Reported: 2014-12-19 06:22 UTC by Lawrence Chen
Modified: 2015-01-22 13:33 UTC (History)
2 users (show)

See Also:
koobs: maintainer-feedback+


Attachments
patch to unbreak what wasn't broke, leaves fix for what was. (597 bytes, patch)
2014-12-19 06:22 UTC, Lawrence Chen
koobs: maintainer-approval? (meta)
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Lawrence Chen 2014-12-19 06:22:08 UTC
Created attachment 150747 [details]
patch to unbreak what wasn't broke, leaves fix for what was.

since x11-server/xorg-server was updated to "Fix multiple xserver security advisories in the 1.12.4 xserver."

net/tigervnc fails to build at this point:

(00:13:42) In file included from ../../include/region.h:51,
(00:13:42)                  from ../../include/window.h:52,
(00:13:42)                  from ../../include/input.h:56,
(00:13:42)                  from ../../hw/xfree86/common/xf86str.h:39,
(00:13:42)                  from ../../hw/xfree86/common/xf86.h:45,
(00:13:42)                  from xf86vncModule.cc:39:
(00:13:42) ../../include/regionstr.h: In function 'void RegionInit(pixman_region16*, pixman_box16*, int)':
(00:13:42) ../../include/regionstr.h:147: error: invalid conversion from 'void*' to 'pixman_region16_data_t*'
(00:13:42) Makefile:893: recipe for target 'libvnc_la-xf86vncModule.lo' failed

After searching online for a while, I came across this message post:

> Re: [tigervnc-devel] Ubuntu trusty build failures
> 
> Alan Coopersmith Thu, 11 Dec 2014 17:34:03 -0800
> 
> On 12/11/14 05:19 PM, Brian Hinz wrote:
> > As of yesterday, upstream patches to the ubuntu 14.04 xorg-server-source package
> > cause our build to fail with the following error:
> > 
> > ../../include/regionstr.h: In function 'void RegionInit(RegionPtr, BoxPtr, 
> > int)':
> > ../../include/regionstr.h:147:45: error: invalid conversion from 'void*' to 
> > 'pixman_region16_data_t* {aka pixman_region16_data*}' [-fpermissive]
> >               (((_pReg)->data = malloc(rgnSize)) != NULL)) {
> > 
> > Still looking into what changed that's now causing this, but thechangelog  
> > <http://changelogs.ubuntu.com/changelogs/pool/main/x/xorg-server/xorg-server_1.15.1-0ubuntu2.5/changelog>
> >   references CVEs so we should probably review them to make sure the 1.4.0 release is 
> > not affected.
> 
> Sorry, I'm a C programmer, so I'm in the habit of deleting casts of malloc()
> results, forgetting that breaks C++.
> 
> After the patches were released, I also found late yesterday that this change
> had broken our  TigerVNC 1.1 package build on Solaris:
> 
> -        if (((_size) > 1) && ((_pReg)->data =
> -                              (RegDataPtr) malloc(RegionSizeof(_size)))) {
> +        if (((_size) > 1) && ((rgnSize = RegionSizeof(_size)) > 0) &&
> +            (((_pReg)->data = malloc(rgnSize)) != NULL)) {
> 
> from http://cgit.freedesktop.org/xorg/xserver/commit/?id=97015a07b9e15d8ec5608b95d95ec0eb51202acb
> 
> I can make it build again by putting the (RegDataPtr) back but was hoping we
> could find some way to make extern "C" { ... } or similar convince the compiler
> C code was okay, since on the upstream Xorg side we have no way of knowing when
> our C changes break VNC trying to use our C code as C++ code.
> 
> --
>         -Alan Coopersmith-              alan.coopersm...@oracle.com
>          Oracle Solaris Engineering - http://blogs.oracle.com/alanc
> _______________________________________________
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel

Well, as a C Programmer...sure....

1> casting is redundant. so not doing it saves typing, how does removing it save?  Not casting (or worse removing somebody else's cast, would get me burned in a code review....but then company had so far failed to get acquire by Oracle.)
2> it would mask failing to include stdlib.h, so make sure its included (of course that would lead to redundancy.)
3> if the type is changed, it require updating all the malloc's.  Unless a typedef is used....which, umm, is what RegDataPtr is.
Comment 1 Bugzilla Automation freebsd_committer freebsd_triage 2014-12-19 06:22:08 UTC
Maintainer CC'd
Comment 2 Koichiro Iwao freebsd_committer freebsd_triage 2014-12-19 09:08:18 UTC
I have just submitted Bug 196118 but it is workaround.
I will look at your patch and consider taking yours, thanks.
Comment 3 John Marino freebsd_committer freebsd_triage 2014-12-19 16:47:32 UTC
Whenever the final patch is decided, is there a possibility to have the poudriere build log uploaded as an attachment?
Comment 4 John Marino freebsd_committer freebsd_triage 2014-12-29 15:44:33 UTC
Is there anything new to report?  I'd be interested in committing a fix, but there's been no progress on these two PRs.
Comment 5 Koichiro Iwao freebsd_committer freebsd_triage 2014-12-30 13:44:23 UTC
I'm working on it now.
Comment 6 Koichiro Iwao freebsd_committer freebsd_triage 2014-12-30 14:58:20 UTC
I like the submitter's patch rather than mine but x11-servers/xorg-server also has been updated to 1.14.7 from 1.12.1, I still have some more to work on.
Comment 7 John Marino freebsd_committer freebsd_triage 2015-01-20 18:17:01 UTC
releasing CC...
There's been no activity and tigervnc has been build for me for last few weeks (from a full bulk run)
Comment 8 Koichiro Iwao freebsd_committer freebsd_triage 2015-01-22 00:27:56 UTC
Now it successfully builds with no changes with tigervnc port.
Comment 9 John Marino freebsd_committer freebsd_triage 2015-01-22 10:22:42 UTC
so we should just close the PR as "can't reproduce" or "overcome by events" ?

In other words, is there anything to do now?
Comment 10 Koichiro Iwao freebsd_committer freebsd_triage 2015-01-22 13:32:45 UTC
Yes, I confirmed it successfully builds and is considered "overcome by events".
However, I am not privileged to close this PR. Can you?
Comment 11 John Marino freebsd_committer freebsd_triage 2015-01-22 13:33:42 UTC
done, thanks.