[UPDATE] textproc/pdfgrep

classic Classic list List threaded Threaded
22 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[UPDATE] textproc/pdfgrep

Reinhold Straub
Hi,

pdfgrep has a testsuite based on devel/dejagnu now. Unfortunately, some patches are necessary to make tests work on OpenBSD.

I put pledge(2) calls into the source code, too.

Regards,
Reinhold Straub


Index: Makefile
===================================================================
RCS file: /cvs/ports/textproc/pdfgrep/Makefile,v
retrieving revision 1.3
diff -u -p -r1.3 Makefile
--- Makefile 25 Aug 2015 16:28:04 -0000 1.3
+++ Makefile 15 Jan 2016 08:27:49 -0000
@@ -1,8 +1,8 @@
-# $OpenBSD: Makefile,v 1.3 2015/08/25 16:28:04 jca Exp $
+# $OpenBSD: Makefile,v 1.4 2015/08/25 16:28:04 jca Exp $
 
 COMMENT = tool to search text in PDF files
 
-DISTNAME = pdfgrep-1.4.0
+DISTNAME = pdfgrep-1.4.1
 
 CATEGORIES = textproc
 
@@ -13,15 +13,16 @@ MAINTAINER = Reinhold Straub <demarchie
 #GPLv2+
 PERMIT_PACKAGE_CDROM = Yes
 
-MODULES = gcc4
-MODGCC4_ARCHS = *
-MODGCC4_LANGS = c++
+MODULES =               gcc4
+MODGCC4_ARCHS =         *
+MODGCC4_LANGS =         c++
 
-WANTLIB += c m pthread stdc++ poppler-cpp pcre
+WANTLIB += c m pthread poppler-cpp pcre
 
 MASTER_SITES = https://pdfgrep.org/download/
 
 LIB_DEPENDS = print/poppler
+TEST_DEPENDS =          devel/dejagnu print/texlive/base
 
 CONFIGURE_ENV = LDFLAGS="-L${X11BASE}/lib"
 CONFIGURE_STYLE = gnu
Index: distinfo
===================================================================
RCS file: /cvs/ports/textproc/pdfgrep/distinfo,v
retrieving revision 1.2
diff -u -p -r1.2 distinfo
--- distinfo 25 Aug 2015 16:28:04 -0000 1.2
+++ distinfo 15 Jan 2016 08:27:49 -0000
@@ -1,2 +1,2 @@
-SHA256 (pdfgrep-1.4.0.tar.gz) = MwwRG5GpIRbVpnZsGx0i3NeVkonXNHHZNEsLBHX8quI=
-SIZE (pdfgrep-1.4.0.tar.gz) = 125269
+SHA256 (pdfgrep-1.4.1.tar.gz) = 2wSiEOa7e3fNbFSxfw9v7Q0SOoX5elQbJwc2pdOEDyw=
+SIZE (pdfgrep-1.4.1.tar.gz) = 151926

patches.tar.gz (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [UPDATE] textproc/pdfgrep

Jiri B-2
On Fri, Jan 15, 2016 at 10:04:36AM +0100, Reinhold Straub wrote:
> Hi,
>
> pdfgrep has a testsuite based on devel/dejagnu now. Unfortunately, some patches are necessary to make tests work on OpenBSD.
>
> I put pledge(2) calls into the source code, too.

So where are patches? Did you forget to cvs add it?

j.

Reply | Threaded
Open this post in threaded view
|

Re: [UPDATE] textproc/pdfgrep

Stuart Henderson-6
In reply to this post by Reinhold Straub
On 2016/01/15 10:04, Reinhold Straub wrote:
> Hi,
>
> pdfgrep has a testsuite based on devel/dejagnu now. Unfortunately, some patches are necessary to make tests work on OpenBSD.
>
> I put pledge(2) calls into the source code, too.

Nice. I think the combination of PCRE and (often untrusted) PDF files
makes this a useful target for pledge.

If people are interested in looking at adding pledge to other ports,
I have a little list of other ports where it might be both a) useful
and b) reasonably sane.

Comments inline:

> -# $OpenBSD: Makefile,v 1.3 2015/08/25 16:28:04 jca Exp $
> +# $OpenBSD: Makefile,v 1.4 2015/08/25 16:28:04 jca Exp $

please leave the $OpenBSD lines alone in diffs, CVS handles them
automatically.

> -MODULES = gcc4
> -MODGCC4_ARCHS = *
> -MODGCC4_LANGS = c++
> +MODULES =               gcc4
> +MODGCC4_ARCHS =         *
> +MODGCC4_LANGS =         c++

this chunk is replacing good whitespace with bad ;)

> +TEST_DEPENDS =          devel/dejagnu print/texlive/base

and please use the standard format here like most other Makefiles
(new line for each dep).

Here's a diff including the above, the patches from your tar.gz in
the diff (which you can't do easily yourself with anoncvs), and with
some other whitespace cleanup. I also added a TEST_FLAGS so we can
see what dejagnu is doing.

There's one test failure in regex.exp: it seems minor so I won't hold
up committing because of this, but do you know what that's about?
It is not related to the pledge patch.

[...]
Running ./pdfgrep.tests/regex.exp ...
PASS: Match line after double match
PASS: Match line after double match -- fixed string
PASS: Match line after double match -- PCRE
FAIL: Empty pattern -- error from pdfgrep
[...]

Anyway it's working fine for me, I've tested with password-protected
files as well (and I can't think of any of the command-line options
that might change required pledges), so if you're still happy with
this I can go ahead and commit.

Index: Makefile
===================================================================
RCS file: /cvs/ports/textproc/pdfgrep/Makefile,v
retrieving revision 1.3
diff -u -p -r1.3 Makefile
--- Makefile 25 Aug 2015 16:28:04 -0000 1.3
+++ Makefile 15 Jan 2016 11:56:14 -0000
@@ -2,7 +2,7 @@
 
 COMMENT = tool to search text in PDF files
 
-DISTNAME = pdfgrep-1.4.0
+DISTNAME = pdfgrep-1.4.1
 
 CATEGORIES = textproc
 
@@ -10,22 +10,25 @@ HOMEPAGE = https://pdfgrep.org/
 
 MAINTAINER = Reinhold Straub <[hidden email]>
 
-#GPLv2+
+# GPLv2+
 PERMIT_PACKAGE_CDROM = Yes
 
 MODULES = gcc4
 MODGCC4_ARCHS = *
 MODGCC4_LANGS = c++
 
-WANTLIB += c m pthread stdc++ poppler-cpp pcre
+WANTLIB += c m pthread poppler-cpp pcre
 
 MASTER_SITES = https://pdfgrep.org/download/
 
 LIB_DEPENDS = print/poppler
+TEST_DEPENDS = devel/dejagnu \
+ print/texlive/base
 
 CONFIGURE_ENV = LDFLAGS="-L${X11BASE}/lib"
 CONFIGURE_STYLE = gnu
 
 MAKE_FLAGS = CXX="${CXX}"
+TEST_FLAGS = RUNTESTFLAGS="--all"
 
 .include <bsd.port.mk>
Index: distinfo
===================================================================
RCS file: /cvs/ports/textproc/pdfgrep/distinfo,v
retrieving revision 1.2
diff -u -p -r1.2 distinfo
--- distinfo 25 Aug 2015 16:28:04 -0000 1.2
+++ distinfo 15 Jan 2016 11:56:14 -0000
@@ -1,2 +1,2 @@
-SHA256 (pdfgrep-1.4.0.tar.gz) = MwwRG5GpIRbVpnZsGx0i3NeVkonXNHHZNEsLBHX8quI=
-SIZE (pdfgrep-1.4.0.tar.gz) = 125269
+SHA256 (pdfgrep-1.4.1.tar.gz) = 2wSiEOa7e3fNbFSxfw9v7Q0SOoX5elQbJwc2pdOEDyw=
+SIZE (pdfgrep-1.4.1.tar.gz) = 151926
Index: patches/patch-src_pdfgrep_cc
===================================================================
RCS file: patches/patch-src_pdfgrep_cc
diff -N patches/patch-src_pdfgrep_cc
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-src_pdfgrep_cc 15 Jan 2016 11:56:14 -0000
@@ -0,0 +1,27 @@
+$OpenBSD$
+--- src/pdfgrep.cc.orig Wed Sep 16 21:06:49 2015
++++ src/pdfgrep.cc Fri Jan 15 11:39:19 2016
+@@ -569,6 +569,11 @@ void handle_poppler_errors(const std::string &msg, voi
+
+ int main(int argc, char** argv)
+ {
++ if (pledge("stdio rpath ioctl", NULL) == -1) {
++ fprintf ( stderr, "pdfgrep: pledge\n" );
++ exit ( 1 );
++ }
++
+ init_colors();
+
+ enum re_engine_type {
+@@ -773,6 +778,11 @@ int main(int argc, char** argv)
+ } else if (context == -2) {
+ // on non-terminals, always print the whole line
+ context = -1;
++ }
++
++ if (pledge("stdio rpath", NULL) == -1) {
++ fprintf ( stderr, "pdfgrep: pledge\n" );
++ exit ( 1 );
+ }
+
+ if (excludes_empty(includes))
Index: patches/patch-testsuite_Makefile_in
===================================================================
RCS file: patches/patch-testsuite_Makefile_in
diff -N patches/patch-testsuite_Makefile_in
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-testsuite_Makefile_in 15 Jan 2016 11:56:14 -0000
@@ -0,0 +1,12 @@
+$OpenBSD$
+--- testsuite/Makefile.in.orig Tue Sep 29 18:18:42 2015
++++ testsuite/Makefile.in Tue Sep 29 18:18:27 2015
+@@ -643,7 +643,7 @@ uninstall-am:
+ .PRECIOUS: Makefile
+
+
+-export DEJAGNU
++# export DEJAGNU
+
+ # Tell versions [3.59,3.63) of GNU make to not export all variables.
+ # Otherwise a system limit (for SysV at least) may be exceeded.
Index: patches/patch-testsuite_lib_pdfgrep_exp
===================================================================
RCS file: patches/patch-testsuite_lib_pdfgrep_exp
diff -N patches/patch-testsuite_lib_pdfgrep_exp
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-testsuite_lib_pdfgrep_exp 15 Jan 2016 11:56:14 -0000
@@ -0,0 +1,12 @@
+$OpenBSD$
+--- testsuite/lib/pdfgrep.exp.orig Wed Jan 13 15:06:57 2016
++++ testsuite/lib/pdfgrep.exp Wed Jan 13 15:12:42 2016
+@@ -213,7 +213,7 @@ proc reset_configuration {} {
+
+ # The directory where the PDFs will be generated.
+ # NOTE This will frequently be removed, so don't put important data there
+-set pdfdir [exec mktemp --tmpdir -d pdfgrep_tests.XXXXXXXXXX]
++set pdfdir [exec mktemp -d /tmp/pdfgrep_tests.XXXXXXXXXX]
+
+
+ # Delete $pdfdir recursively and create it anew
Index: patches/patch-testsuite_pdfgrep_tests_exit_status_exp
===================================================================
RCS file: patches/patch-testsuite_pdfgrep_tests_exit_status_exp
diff -N patches/patch-testsuite_pdfgrep_tests_exit_status_exp
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-testsuite_pdfgrep_tests_exit_status_exp 15 Jan 2016 11:56:14 -0000
@@ -0,0 +1,19 @@
+$OpenBSD$
+--- testsuite/pdfgrep.tests/exit_status.exp.orig Wed Jan 13 15:07:29 2016
++++ testsuite/pdfgrep.tests/exit_status.exp Wed Jan 13 15:10:27 2016
+@@ -8,7 +8,7 @@ clear_pdfdir
+ set pdf [mkpdf exit-status "foobar"]
+
+ pdfgrep foobar $pdf
+-
++expect eof
+ expect_exit_status 0
+
+ ########################################
+@@ -40,5 +40,5 @@ clear_pdfdir
+
+ # $pdf doesn't exist anymore
+ pdfgrep foobar $pdf
+-
++expect eof
+ expect_exit_status 2
Index: patches/patch-testsuite_pdfgrep_tests_usage_exp
===================================================================
RCS file: patches/patch-testsuite_pdfgrep_tests_usage_exp
diff -N patches/patch-testsuite_pdfgrep_tests_usage_exp
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-testsuite_pdfgrep_tests_usage_exp 15 Jan 2016 11:56:14 -0000
@@ -0,0 +1,12 @@
+$OpenBSD$
+--- testsuite/pdfgrep.tests/usage.exp.orig Wed Jan 13 15:07:55 2016
++++ testsuite/pdfgrep.tests/usage.exp Wed Jan 13 15:11:50 2016
+@@ -5,7 +5,7 @@ expect {
+     -re "^Usage: .*" { pass $test }
+     default { fail $test }
+ }
+-
++expect eof
+ expect_exit_status 2
+
+ # Also look that nothing is written to stdout

Reply | Threaded
Open this post in threaded view
|

Re: [UPDATE] textproc/pdfgrep

Dmitrij D. Czarkoff-2
In reply to this post by Reinhold Straub
One test still fails for me:

  Running ./pdfgrep.tests/regex.exp ...
  FAIL: Empty pattern -- error from pdfgrep

Indeed, when pdfgrep is called with argument "", instead of matching
every line it exits with error

  pdfgrep: empty (sub)expression

This happens because our regex(3) doesn't allow empty patterns.  As far
as I can tell, this behavior is undesirable in something called "*grep",
so I added a quirk for this particular use case.

Other comments:

In Makefile:

> -MODULES = gcc4
> -MODGCC4_ARCHS = *
> -MODGCC4_LANGS = c++
> +MODULES =               gcc4
> +MODGCC4_ARCHS =         *
> +MODGCC4_LANGS =         c++

Whitespace change.  Why?

> +TEST_DEPENDS =          devel/dejagnu print/texlive/base

Again, expanded space.


In patches/patch-src-pdfgrep.cc:
| +        if (pledge("stdio rpath ioctl", NULL) == -1) {
| +               fprintf ( stderr, "pdfgrep: pledge\n" );
| +               exit ( 1 );
| +        }
| +
[...]
| +
| +        if (pledge("stdio rpath", NULL) == -1) {
| +               fprintf ( stderr, "pdfgrep: pledge\n" );
| +               exit ( 1 );
| +        }

I don't see a point in first pledge.  As a matter of fact, I wouldn't
pledge this port unless upstream is ready to accept pledge.  And
upstream will definitely not accept pledge without #ifdef.


In patches/patch-testsuite-lib-pdfgrep.exp:
| -set pdfdir [exec mktemp --tmpdir -d pdfgrep_tests.XXXXXXXXXX]
| +set pdfdir [exec mktemp -d /tmp/pdfgrep_tests.XXXXXXXXXX]

I'd use "mktemp -td pdfgrep_tests.XXXXXXXXXX]": this form still allows
altering temporary directory, and upstream may want to use portable
syntax.


Patch with my corrections follow.  I left second pledge in case people
want it, although it will take much more work to make this patch
upstreamable.

--
Dmitrij D. Czarkoff

--- Makefile.orig Tue Aug 25 18:28:04 2015
+++ Makefile Fri Jan 15 12:28:50 2016
@@ -1,8 +1,8 @@
-# $OpenBSD: Makefile,v 1.3 2015/08/25 16:28:04 jca Exp $
+# $OpenBSD: Makefile,v 1.4 2015/08/25 16:28:04 jca Exp $
 
 COMMENT = tool to search text in PDF files
 
-DISTNAME = pdfgrep-1.4.0
+DISTNAME = pdfgrep-1.4.1
 
 CATEGORIES = textproc
 
@@ -17,11 +17,12 @@
 MODGCC4_ARCHS = *
 MODGCC4_LANGS = c++
 
-WANTLIB += c m pthread stdc++ poppler-cpp pcre
+WANTLIB += c m pthread poppler-cpp pcre
 
 MASTER_SITES = https://pdfgrep.org/download/
 
 LIB_DEPENDS = print/poppler
+TEST_DEPENDS = devel/dejagnu print/texlive/base
 
 CONFIGURE_ENV = LDFLAGS="-L${X11BASE}/lib"
 CONFIGURE_STYLE = gnu
--- distinfo.orig Tue Aug 25 18:28:04 2015
+++ distinfo Fri Jan 15 10:35:39 2016
@@ -1,2 +1,2 @@
-SHA256 (pdfgrep-1.4.0.tar.gz) = MwwRG5GpIRbVpnZsGx0i3NeVkonXNHHZNEsLBHX8quI=
-SIZE (pdfgrep-1.4.0.tar.gz) = 125269
+SHA256 (pdfgrep-1.4.1.tar.gz) = 2wSiEOa7e3fNbFSxfw9v7Q0SOoX5elQbJwc2pdOEDyw=
+SIZE (pdfgrep-1.4.1.tar.gz) = 151926
--- patches/patch-src_pdfgrep_cc.orig Fri Jan 15 13:19:30 2016
+++ patches/patch-src_pdfgrep_cc Fri Jan 15 12:44:28 2016
@@ -0,0 +1,15 @@
+$OpenBSD$
+--- src/pdfgrep.cc.orig Wed Sep 16 22:06:49 2015
++++ src/pdfgrep.cc Fri Jan 15 12:29:06 2016
+@@ -775,6 +775,11 @@ int main(int argc, char** argv)
+ context = -1;
+ }
+
++ if (pledge("stdio rpath", NULL) == -1) {
++ fprintf(stderr, "pdfgrep: pledge\n");
++ exit(1);
++ }
++
+ if (excludes_empty(includes))
+ exclude_add(includes, "*.pdf");
+
--- patches/patch-src_regengine_cc.orig Fri Jan 15 13:19:30 2016
+++ patches/patch-src_regengine_cc Fri Jan 15 13:07:54 2016
@@ -0,0 +1,13 @@
+$OpenBSD$
+--- src/regengine.cc.orig Fri Jan 15 13:06:26 2016
++++ src/regengine.cc Fri Jan 15 13:07:45 2016
+@@ -34,6 +34,9 @@ PosixRegex::PosixRegex(const char *pattern, bool case_
+ {
+ int regex_flags = REG_EXTENDED | (case_insensitive ? REG_ICASE : 0);
+
++ if (strncmp(pattern, "", 2) == 0) {
++ pattern = "()";
++ }
+ int err = regcomp(&this->regex, pattern, regex_flags);
+ if(err) {
+ char err_msg[256];
--- patches/patch-testsuite_Makefile_in.orig Fri Jan 15 13:19:30 2016
+++ patches/patch-testsuite_Makefile_in Fri Jan 15 13:19:11 2016
@@ -0,0 +1,14 @@
+$OpenBSD$
+--- testsuite/Makefile.in.orig Thu Sep 24 21:32:33 2015
++++ testsuite/Makefile.in Fri Jan 15 13:18:59 2016
+@@ -641,10 +641,3 @@ uninstall-am:
+ tags-am uninstall uninstall-am
+
+ .PRECIOUS: Makefile
+-
+-
+-export DEJAGNU
+-
+-# Tell versions [3.59,3.63) of GNU make to not export all variables.
+-# Otherwise a system limit (for SysV at least) may be exceeded.
+-.NOEXPORT:
--- patches/patch-testsuite_lib_pdfgrep_exp.orig Fri Jan 15 13:19:30 2016
+++ patches/patch-testsuite_lib_pdfgrep_exp Fri Jan 15 11:12:21 2016
@@ -0,0 +1,12 @@
+$OpenBSD$
+--- testsuite/lib/pdfgrep.exp.orig Wed Jan 13 15:06:57 2016
++++ testsuite/lib/pdfgrep.exp Wed Jan 13 15:12:42 2016
+@@ -213,7 +213,7 @@ proc reset_configuration {} {
+
+ # The directory where the PDFs will be generated.
+ # NOTE This will frequently be removed, so don't put important data there
+-set pdfdir [exec mktemp --tmpdir -d pdfgrep_tests.XXXXXXXXXX]
++set pdfdir [exec mktemp -td pdfgrep_tests.XXXXXXXXXX]
+
+
+ # Delete $pdfdir recursively and create it anew
--- patches/patch-testsuite_pdfgrep_tests_exit_status_exp.orig Fri Jan 15 13:19:30 2016
+++ patches/patch-testsuite_pdfgrep_tests_exit_status_exp Fri Jan 15 09:26:05 2016
@@ -0,0 +1,19 @@
+$OpenBSD$
+--- testsuite/pdfgrep.tests/exit_status.exp.orig Wed Jan 13 15:07:29 2016
++++ testsuite/pdfgrep.tests/exit_status.exp Wed Jan 13 15:10:27 2016
+@@ -8,7 +8,7 @@ clear_pdfdir
+ set pdf [mkpdf exit-status "foobar"]
+
+ pdfgrep foobar $pdf
+-
++expect eof
+ expect_exit_status 0
+
+ ########################################
+@@ -40,5 +40,5 @@ clear_pdfdir
+
+ # $pdf doesn't exist anymore
+ pdfgrep foobar $pdf
+-
++expect eof
+ expect_exit_status 2
--- patches/patch-testsuite_pdfgrep_tests_usage_exp.orig Fri Jan 15 13:19:30 2016
+++ patches/patch-testsuite_pdfgrep_tests_usage_exp Fri Jan 15 09:26:05 2016
@@ -0,0 +1,12 @@
+$OpenBSD$
+--- testsuite/pdfgrep.tests/usage.exp.orig Wed Jan 13 15:07:55 2016
++++ testsuite/pdfgrep.tests/usage.exp Wed Jan 13 15:11:50 2016
+@@ -5,7 +5,7 @@ expect {
+     -re "^Usage: .*" { pass $test }
+     default { fail $test }
+ }
+-
++expect eof
+ expect_exit_status 2
+
+ # Also look that nothing is written to stdout

Reply | Threaded
Open this post in threaded view
|

Re: [UPDATE] textproc/pdfgrep

Theo Buehler
In reply to this post by Stuart Henderson-6
> + int main(int argc, char** argv)
> + {
> ++ if (pledge("stdio rpath ioctl", NULL) == -1) {
> ++ fprintf ( stderr, "pdfgrep: pledge\n" );
> ++ exit ( 1 );
> ++ }

This ioctl pledge feels wrong.  I only have a lousy internet connection
and no gcc 4.9 installed, so I can't really test.  The source shows only
a TIOCGWINSZ ioctl, which is covered by pledge "tty" which makes much
more sense to me.

Reply | Threaded
Open this post in threaded view
|

Re: [UPDATE] textproc/pdfgrep

Stuart Henderson-6
On 2016/01/15 13:36, Theo Buehler wrote:

> > + int main(int argc, char** argv)
> > + {
> > ++ if (pledge("stdio rpath ioctl", NULL) == -1) {
> > ++ fprintf ( stderr, "pdfgrep: pledge\n" );
> > ++ exit ( 1 );
> > ++ }
>
> This ioctl pledge feels wrong.  I only have a lousy internet connection
> and no gcc 4.9 installed, so I can't really test.  The source shows only
> a TIOCGWINSZ ioctl, which is covered by pledge "tty" which makes much
> more sense to me.
>

Ah yes you are right. I've adjusted it in my tree.

Reply | Threaded
Open this post in threaded view
|

Re: [UPDATE] textproc/pdfgrep

Stuart Henderson-6
In reply to this post by Dmitrij D. Czarkoff-2
On 2016/01/15 13:21, Dmitrij D. Czarkoff wrote:
> This happens because our regex(3) doesn't allow empty patterns.  As far
> as I can tell, this behavior is undesirable in something called "*grep",
> so I added a quirk for this particular use case.

Agreed.

Reply | Threaded
Open this post in threaded view
|

Re: [UPDATE] textproc/pdfgrep

Dmitrij D. Czarkoff-2
In reply to this post by Stuart Henderson-6
Stuart Henderson said:

> On 2016/01/15 13:36, Theo Buehler wrote:
> > > + int main(int argc, char** argv)
> > > + {
> > > ++ if (pledge("stdio rpath ioctl", NULL) == -1) {
> > > ++ fprintf ( stderr, "pdfgrep: pledge\n" );
> > > ++ exit ( 1 );
> > > ++ }
> >
> > This ioctl pledge feels wrong.  I only have a lousy internet connection
> > and no gcc 4.9 installed, so I can't really test.  The source shows only
> > a TIOCGWINSZ ioctl, which is covered by pledge "tty" which makes much
> > more sense to me.
> >
>
> Ah yes you are right. I've adjusted it in my tree.

What is the point of this first pledge call?

--
Dmitrij D. Czarkoff

Reply | Threaded
Open this post in threaded view
|

Re: [UPDATE] textproc/pdfgrep

Stuart Henderson-6
On 2016/01/15 14:42, Dmitrij D. Czarkoff wrote:

> Stuart Henderson said:
> > On 2016/01/15 13:36, Theo Buehler wrote:
> > > > + int main(int argc, char** argv)
> > > > + {
> > > > ++ if (pledge("stdio rpath ioctl", NULL) == -1) {
> > > > ++ fprintf ( stderr, "pdfgrep: pledge\n" );
> > > > ++ exit ( 1 );
> > > > ++ }
> > >
> > > This ioctl pledge feels wrong.  I only have a lousy internet connection
> > > and no gcc 4.9 installed, so I can't really test.  The source shows only
> > > a TIOCGWINSZ ioctl, which is covered by pledge "tty" which makes much
> > > more sense to me.
> > >
> >
> > Ah yes you are right. I've adjusted it in my tree.
>
> What is the point of this first pledge call?

It means that the list of available system calls is cut right down
before the first function calls into PCRE and Poppler, which I think
is nice to have.

I'm pleased to see that Poppler doesn't seem to be having problems
running with pledge for basic use. Hopefully this can be pushed out
into a full PDF viewer.

Reply | Threaded
Open this post in threaded view
|

pledge in ports

Stuart Henderson-6
In reply to this post by Stuart Henderson-6
On 2016/01/15 12:11, Stuart Henderson wrote:
> If people are interested in looking at adding pledge to other ports,
> I have a little list of other ports where it might be both a) useful
> and b) reasonably sane.

Since there was some interest off-list, here's my current list - there
are varying levels of difficulty.

Note that this is a personal list, I haven't talked to maintainers of
the various ports about their thoughts on this and in some cases they'll
have better information than me about the sanity level and whether it's
likely to cause big problems at update, so anyone thinking at looking
at these, please do communicate with maintainers.

Also note it would be best to gain experience with simpler programs
before attempting anything complex (for software with lots of options,
it's difficult to get decent test coverage).

archivers/p7zip
archivers/xz (see cvs log for the previous failed experiment)
mail/mutt
misc/memcached
net/arp-scan
net/avahi
net/bwm-ng or some other bandwidth monitor
net/curl
net/cvsync
net/ladvd and/or net/lldpd
net/mosh
net/ngrep
net/openvpn
net/rsync (N.B. setsockopt)
net/scamper
net/tcptraceroute (or even better, TCP support in traceroute(1) instead;
  I have a start at a diff but never got checksums to work properly)
net/wireshark (dumpcap should be fairly easy as it uses libcap on linux.
  main program would be nicer as this runs the dangerous dissectors).
print/cups
security/clamav
textproc/mupdf
textproc/xpdf  (maybe other pdf viewers - zathura etc)
www/lynx
www/nginx  (at least until they add dlopen module support..)
x11/dbus
x11/rxvt-unicode

Reply | Threaded
Open this post in threaded view
|

Re: pledge in ports

Jiri B-2
On Sat, Jan 16, 2016 at 12:36:49PM +0000, Stuart Henderson wrote:

> archivers/p7zip
> archivers/xz (see cvs log for the previous failed experiment)
> mail/mutt
> misc/memcached
> net/arp-scan
> net/avahi
> net/bwm-ng or some other bandwidth monitor
> net/curl
> net/cvsync
> net/ladvd and/or net/lldpd
> net/mosh
> net/ngrep
> net/openvpn
> net/rsync (N.B. setsockopt)
> net/scamper
> net/tcptraceroute (or even better, TCP support in traceroute(1) instead;
>   I have a start at a diff but never got checksums to work properly)
> net/wireshark (dumpcap should be fairly easy as it uses libcap on linux.
>   main program would be nicer as this runs the dangerous dissectors).
> print/cups
> security/clamav
> textproc/mupdf
> textproc/xpdf  (maybe other pdf viewers - zathura etc)
> www/lynx
> www/nginx  (at least until they add dlopen module support..)
> x11/dbus
> x11/rxvt-unicode

Could you consider Tor please?

j.

Reply | Threaded
Open this post in threaded view
|

Re: pledge in ports

Theo de Raadt
> On Sat, Jan 16, 2016 at 12:36:49PM +0000, Stuart Henderson wrote:
> > archivers/p7zip
> > archivers/xz (see cvs log for the previous failed experiment)
> > mail/mutt
> > misc/memcached
> > net/arp-scan
> > net/avahi
> > net/bwm-ng or some other bandwidth monitor
> > net/curl
> > net/cvsync
> > net/ladvd and/or net/lldpd
> > net/mosh
> > net/ngrep
> > net/openvpn
> > net/rsync (N.B. setsockopt)
> > net/scamper
> > net/tcptraceroute (or even better, TCP support in traceroute(1) instead;
> >   I have a start at a diff but never got checksums to work properly)
> > net/wireshark (dumpcap should be fairly easy as it uses libcap on linux.
> >   main program would be nicer as this runs the dangerous dissectors).
> > print/cups
> > security/clamav
> > textproc/mupdf
> > textproc/xpdf  (maybe other pdf viewers - zathura etc)
> > www/lynx
> > www/nginx  (at least until they add dlopen module support..)
> > x11/dbus
> > x11/rxvt-unicode
>
> Could you consider Tor please?

It appears you misunderstood the purpose of this mail.

You could consider Tor.

Understand the mail now?

Reply | Threaded
Open this post in threaded view
|

Re: pledge in ports

Christian Weisgerber
In reply to this post by Stuart Henderson-6
On 2016-01-16, Stuart Henderson <[hidden email]> wrote:

> archivers/xz (see cvs log for the previous failed experiment)

Oops.

I think I forgot to commit this:
https://marc.info/?l=openbsd-ports&m=144544207928404&w=2

--
Christian "naddy" Weisgerber                          [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: pledge in ports

Josh Grosse
In reply to this post by Stuart Henderson-6
On Sat, Jan 16, 2016 at 12:36:49PM +0000, Stuart Henderson wrote:

> archivers/p7zip

The key module is 80K lines of undocumented code, spread across 219 files.
But it does have a test suite, so I'll see what I can do.

   -Josh-

Reply | Threaded
Open this post in threaded view
|

Re: pledge in ports: textproc/mupdf

Sebastien Marie-2
In reply to this post by Stuart Henderson-6
Hi,

Here a diff for adding pledge(2) to textproc/mupdf. I added ports@ in Cc
in order to get wider reviewing.

I pledged all programs inside textproc/mupdf:
  - mupdf-x11
  - mupdf-x11-curl
  - mupdf-gl
  - mutool draw,clean,extract,info,pages,poster,show
  - mujstest

The patch files explains "unusual" promises or specials cases.

Thanks.
--
Sebastien Marie

Index: Makefile
===================================================================
RCS file: /cvs/ports/textproc/mupdf/Makefile,v
retrieving revision 1.59
diff -u -p -r1.59 Makefile
--- Makefile 12 Nov 2015 17:26:54 -0000 1.59
+++ Makefile 17 Jan 2016 13:23:42 -0000
@@ -8,6 +8,7 @@ COMMENT = graphic library, pdf parser, v
 V = 1.8
 DISTNAME = mupdf-$V-source
 PKGNAME = mupdf-$V
+REVISION = 0
 
 CATEGORIES = textproc x11
 
@@ -23,6 +24,7 @@ PERMIT_PACKAGE_CDROM = Yes
 FLAVORS= js
 FLAVOR?=
 
+# uses pledge(2)
 WANTLIB += GL X11 Xcursor Xext Xinerama Xrandr c crypto curl freetype
 WANTLIB += idn jbig2dec jpeg m nghttp2 openjp2 pthread ssl z
 
Index: patches/patch-platform_gl_gl-main_c
===================================================================
RCS file: patches/patch-platform_gl_gl-main_c
diff -N patches/patch-platform_gl_gl-main_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-platform_gl_gl-main_c 17 Jan 2016 13:23:42 -0000
@@ -0,0 +1,21 @@
+$OpenBSD$
+
+add pledge(2) to mupdf-gl:
+  - drm : opengl stuff
+  - proc exec : opening a external browser on uri link
+
+--- platform/gl/gl-main.c.orig Tue Nov 10 17:19:51 2015
++++ platform/gl/gl-main.c Sun Jan 17 10:21:44 2016
+@@ -1361,6 +1361,12 @@ int main(int argc, char **argv)
+
+ glfwMakeContextCurrent(window);
+
++ if (pledge("stdio rpath drm proc exec", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++
+ ctx = fz_new_context(NULL, NULL, 0);
+ fz_register_document_handlers(ctx);
+
Index: patches/patch-platform_x11_jstest_main_c
===================================================================
RCS file: patches/patch-platform_x11_jstest_main_c
diff -N patches/patch-platform_x11_jstest_main_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-platform_x11_jstest_main_c 17 Jan 2016 13:23:42 -0000
@@ -0,0 +1,21 @@
+$OpenBSD$
+
+add pledge(2) to mujstest
+  - rpath : OPEN command
+  - wpath cpath : SCREENSHOT command
+
+--- platform/x11/jstest_main.c.orig Sun Jan 17 13:50:58 2016
++++ platform/x11/jstest_main.c Sun Jan 17 13:52:29 2016
+@@ -310,6 +310,12 @@ main(int argc, char *argv[])
+ if (fz_optind == argc)
+ usage();
+
++ if (pledge("stdio rpath wpath cpath", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++
+ ctx = fz_new_context(NULL, NULL, FZ_STORE_DEFAULT);
+ if (!ctx)
+ {
Index: patches/patch-platform_x11_x11_main_c
===================================================================
RCS file: patches/patch-platform_x11_x11_main_c
diff -N patches/patch-platform_x11_x11_main_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-platform_x11_x11_main_c 17 Jan 2016 13:23:42 -0000
@@ -0,0 +1,29 @@
+$OpenBSD$
+
+add pledge(2) to mupdf-x11 and mupdf-x11-curl:
+  - inet dns : with HAVE_CURL only
+  - proc exec : opening an external browser on uri link
+
+--- platform/x11/x11_main.c.orig Tue Nov 10 17:19:51 2015
++++ platform/x11/x11_main.c Sun Jan 17 12:19:55 2016
+@@ -885,6 +885,20 @@ int main(int argc, char **argv)
+ tmo_at.tv_usec = 0;
+ timeout = NULL;
+
++#ifdef HAVE_CURL
++ if (pledge("stdio rpath inet dns proc exec", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++#else
++ if (pledge("stdio rpath proc exec", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++#endif
++
+ pdfapp_open(&gapp, filename, 0);
+
+ FD_ZERO(&fds);
Index: patches/patch-source_tools_mudraw_c
===================================================================
RCS file: patches/patch-source_tools_mudraw_c
diff -N patches/patch-source_tools_mudraw_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-source_tools_mudraw_c 17 Jan 2016 13:23:42 -0000
@@ -0,0 +1,31 @@
+$OpenBSD$
+
+add pledge(2) to "mutool draw":
+  - wpath cpath : only if output is specified
+
+--- source/tools/mudraw.c.orig Tue Nov 10 17:19:51 2015
++++ source/tools/mudraw.c Sun Jan 17 10:21:04 2016
+@@ -909,6 +909,23 @@ int mudraw_main(int argc, char **argv)
+ if (fz_optind == argc)
+ usage();
+
++ if (output && output[0] != '-' && *output != 0)
++ {
++ if (pledge("stdio rpath wpath cpath", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++ }
++ else
++ {
++ if (pledge("stdio rpath", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++ }
++
+ ctx = fz_new_context((showmemory == 0 ? NULL : &alloc_ctx), NULL, FZ_STORE_DEFAULT);
+ if (!ctx)
+ {
Index: patches/patch-source_tools_pdfclean_c
===================================================================
RCS file: patches/patch-source_tools_pdfclean_c
diff -N patches/patch-source_tools_pdfclean_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-source_tools_pdfclean_c 17 Jan 2016 13:23:42 -0000
@@ -0,0 +1,19 @@
+$OpenBSD$
+
+add pledge(2) to "mutool clean"
+
+--- source/tools/pdfclean.c.orig Sun Jan 17 11:59:42 2016
++++ source/tools/pdfclean.c Sun Jan 17 12:01:42 2016
+@@ -79,6 +79,12 @@ int pdfclean_main(int argc, char **argv)
+ outfile = argv[fz_optind++];
+ }
+
++ if (pledge("stdio rpath wpath cpath", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++
+ ctx = fz_new_context(NULL, NULL, FZ_STORE_UNLIMITED);
+ if (!ctx)
+ {
Index: patches/patch-source_tools_pdfextract_c
===================================================================
RCS file: patches/patch-source_tools_pdfextract_c
diff -N patches/patch-source_tools_pdfextract_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-source_tools_pdfextract_c 17 Jan 2016 13:23:42 -0000
@@ -0,0 +1,19 @@
+$OpenBSD$
+
+add pledge(2) to "mutool extract"
+
+--- source/tools/pdfextract.c.orig Sun Jan 17 12:12:27 2016
++++ source/tools/pdfextract.c Sun Jan 17 12:13:42 2016
+@@ -202,6 +202,12 @@ int pdfextract_main(int argc, char **argv)
+
+ infile = argv[fz_optind++];
+
++ if (pledge("stdio rpath wpath cpath", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++
+ ctx = fz_new_context(NULL, NULL, FZ_STORE_UNLIMITED);
+ if (!ctx)
+ {
Index: patches/patch-source_tools_pdfinfo_c
===================================================================
RCS file: patches/patch-source_tools_pdfinfo_c
diff -N patches/patch-source_tools_pdfinfo_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-source_tools_pdfinfo_c 17 Jan 2016 13:23:42 -0000
@@ -0,0 +1,19 @@
+$OpenBSD$
+
+add pledge(2) to "mutool info"
+
+--- source/tools/pdfinfo.c.orig Sun Jan 17 10:20:26 2016
++++ source/tools/pdfinfo.c Sun Jan 17 10:23:26 2016
+@@ -1061,6 +1061,12 @@ int pdfinfo_main(int argc, char **argv)
+ if (fz_optind == argc)
+ infousage();
+
++ if (pledge("stdio rpath", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++
+ ctx = fz_new_context(NULL, NULL, FZ_STORE_UNLIMITED);
+ if (!ctx)
+ {
Index: patches/patch-source_tools_pdfpages_c
===================================================================
RCS file: patches/patch-source_tools_pdfpages_c
diff -N patches/patch-source_tools_pdfpages_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-source_tools_pdfpages_c 17 Jan 2016 13:23:42 -0000
@@ -0,0 +1,19 @@
+$OpenBSD$
+
+add pledge(2) to "mutool pages"
+
+--- source/tools/pdfpages.c.orig Sun Jan 17 12:13:56 2016
++++ source/tools/pdfpages.c Sun Jan 17 12:15:06 2016
+@@ -229,6 +229,12 @@ int pdfpages_main(int argc, char **argv)
+ if (fz_optind == argc)
+ infousage();
+
++ if (pledge("stdio rpath", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++
+ ctx = fz_new_context(NULL, NULL, FZ_STORE_UNLIMITED);
+ if (!ctx)
+ {
Index: patches/patch-source_tools_pdfposter_c
===================================================================
RCS file: patches/patch-source_tools_pdfposter_c
diff -N patches/patch-source_tools_pdfposter_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-source_tools_pdfposter_c 17 Jan 2016 13:23:42 -0000
@@ -0,0 +1,19 @@
+$OpenBSD$
+
+add pledge(2) to "mutool poster"
+
+--- source/tools/pdfposter.c.orig Sun Jan 17 12:21:27 2016
++++ source/tools/pdfposter.c Sun Jan 17 12:22:56 2016
+@@ -189,6 +189,12 @@ int pdfposter_main(int argc, char **argv)
+ outfile = argv[fz_optind++];
+ }
+
++ if (pledge("stdio rpath wpath cpath", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++
+ ctx = fz_new_context(NULL, NULL, FZ_STORE_UNLIMITED);
+ if (!ctx)
+ {
Index: patches/patch-source_tools_pdfshow_c
===================================================================
RCS file: patches/patch-source_tools_pdfshow_c
diff -N patches/patch-source_tools_pdfshow_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-source_tools_pdfshow_c 17 Jan 2016 13:23:42 -0000
@@ -0,0 +1,35 @@
+$OpenBSD$
+
+add pledge(2) to "mutool show":
+  - rpath : dropped after opening
+  - no need of wpath cpath for -o, as the open is already done at this place
+
+--- source/tools/pdfshow.c.orig Sun Jan 17 09:08:52 2016
++++ source/tools/pdfshow.c Sun Jan 17 09:15:05 2016
+@@ -247,6 +247,12 @@ int pdfshow_main(int argc, char **argv)
+ }
+ }
+
++ if (pledge("stdio rpath", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++
+ ctx = fz_new_context(NULL, NULL, FZ_STORE_UNLIMITED);
+ if (!ctx)
+ {
+@@ -258,6 +264,13 @@ int pdfshow_main(int argc, char **argv)
+ fz_try(ctx)
+ {
+ doc = pdf_open_document(ctx, filename);
++
++ if (pledge("stdio", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++
+ if (pdf_needs_password(ctx, doc))
+ if (!pdf_authenticate_password(ctx, doc, password))
+ fz_warn(ctx, "cannot authenticate password: %s", filename);

Reply | Threaded
Open this post in threaded view
|

Re: pledge in ports

Stuart Henderson-6
In reply to this post by Josh Grosse
On 2016/01/17 08:59, Josh Grosse wrote:
> On Sat, Jan 16, 2016 at 12:36:49PM +0000, Stuart Henderson wrote:
>
> > archivers/p7zip
>
> The key module is 80K lines of undocumented code, spread across 219 files.
> But it does have a test suite, so I'll see what I can do.

Yikes! But on the plus side, a lot of that is computation rather than
system interface, and the way it's been split up to deal with the
different front-end UIs and platforms should help isolate the parts of
code that deal with the system.

As far as I'm aware it doesn't do network access itself or execute other
programs, and even a pledge that only prevents those things is very
meaningful.

If you have chance to try that would be great - please do post if you
run into problems or have any questions.

Reply | Threaded
Open this post in threaded view
|

Re: pledge in ports

Josh Grosse
On Sun, Jan 17, 2016 at 02:38:47PM +0000, Stuart Henderson wrote:
> As far as I'm aware it doesn't do network access itself or execute other
> programs, and even a pledge that only prevents those things is very
> meaningful.
>
> If you have chance to try that would be great - please do post if you
> run into problems or have any questions.

Thank you for the offer of assistance, Stuart.  
 
I've begun looking into it.  Reading kdump output from a test is far,
far easier than reading the actual code.  :)  

I'm sorting the syscalls used in the 7za test now.  Once classified into
promise groups, I'll see if I can add pledge to it.

This is an easier task than trying to figure out why gdb(1) gives me
random values whenever I ask for the structure referenced  by a pointer
that points outside of the process map in a .core file.  Which is what
I was doing for 4 hours last evening.  I'm slow.  I spent 3.75 of those
hours trying to figure out what was wrong with the .core file, before
discovering egdb didn't have the problem.  :)

Reply | Threaded
Open this post in threaded view
|

Re: pledge in ports

Carlin Bingham
In reply to this post by Jiri B-2
On Sun, 17 Jan 2016, at 04:13 AM, Jiri B wrote:
>
> Could you consider Tor please?
>
> j.
>

tor's pledge will looking something like:

      pledge("stdio rpath cpath wpath ps id dns inet unix flock getpw
      proc exec pf", NULL)


None of these can be dropped later or made conditional on the
configuration, as tor's config can be changed and reloaded while it's
running and it needs them all to handle that.

Is a wide pledge like this still beneficial?


Explanation for these:

stdio - is obvious
rpath, cpath, wpath - reading/creating/writing cached descriptors etc.
(also logging without syslog)
ps - uses sysctl to decide resource limits if they're not defined in
torrc
id - sets rlimits
dns - obvious
inet - tor needs sockets
unix - unix sockets can be used for the socks and control ports
flock - locking file to prevent multiple instances writing the data dir
getpw - to drop privs, chown unix sockets, answer GETINFO commands to
control port
proc - daemonising
exec - daemonising and pluggable transports
pf - this could be ifdef'd, only needed if transparent proxying to pf is
enabled when tor is built

--
Carlin

Reply | Threaded
Open this post in threaded view
|

Re: pledge in ports: textproc/mupdf

Stuart Henderson-6
In reply to this post by Sebastien Marie-2
On 2016/01/17 15:22, Sebastien Marie wrote:

> Hi,
>
> Here a diff for adding pledge(2) to textproc/mupdf. I added ports@ in Cc
> in order to get wider reviewing.
>
> I pledged all programs inside textproc/mupdf:
>   - mupdf-x11
>   - mupdf-gl
>   - mutool draw,clean,extract,info,pages,poster,show
>   - mujstest

These seem fine in my tests so far (tried a few files with various
image formats etc and some with passwords).

>   - mupdf-x11-curl

I ran into a problem with mupdf-x11-curl fetching from http getting
killed on the first file I tried: as luck would have it I haven't been
able to repeat it with another file, but http://dmtx.uk/ugh.pdf
if you'd like to try it. Works ok from disk. It fails to display
in the unpledged version too (sits burning cpu), so there's an
upstream bug anyway.

I've run out of time to dig for now but here's what I have so far:

mupdf: warning: not enough data to open yet
error: read of a block we don't have (E) (offset=0)
error: cannot load document from stream
[...lots of the above...]
mupdf: warning: not enough data to open yet
error: cannot find object in xref (8150 0 R) - not loaded yet?
error: cannot find object in xref (8150 0 R) - not loaded yet?
error: cannot find object in xref (18 0 R) - not loaded yet?
Pledged version - Abort trap (core dumped)
Unpledged version - warning: Background fetch complete!

Core was generated by `mupdf-x11-curl'.
Program terminated with signal SIGABRT, Aborted.
#0  0x00000dc8f939e4da in socket () at <stdin>:2
2 <stdin>: No such file or directory.
[Current thread is 1 (process 6470)]
(gdb) bt
#0  0x00000dc8f939e4da in socket () at <stdin>:2
#1  0x00000dc852af5bbb in _xcb_socket (family=1, type=1, proto=0)
    at /usr/xenocara/lib/libxcb/libxcb/../../../dist/libxcb/src/xcb_util.c:315
#2  0x00000dc852af5f7a in _xcb_open_unix (file=<optimized out>, protocol=0x0)
    at /usr/xenocara/lib/libxcb/libxcb/../../../dist/libxcb/src/xcb_util.c:440
#3  _xcb_open (display=0, protocol=<optimized out>, host=0xdc89f4ded90 "")
    at /usr/xenocara/lib/libxcb/libxcb/../../../dist/libxcb/src/xcb_util.c:297
#4  xcb_connect_to_display_with_auth_info (displayname=<optimized out>, auth=0x0,
    screenp=0x0)
    at /usr/xenocara/lib/libxcb/libxcb/../../../dist/libxcb/src/xcb_util.c:506
#5  0x00000dc85d15060b in _XConnectXCB () from /usr/X11R6/lib/libX11.so.16.1
#6  0x00000dc85d1414fb in XOpenDisplay () from /usr/X11R6/lib/libX11.so.16.1
#7  0x00000dc60370f4e0 in winreloadpage (app=<optimized out>)
    at platform/x11/x11_main.c:701
#8  0x00000dc603717f8b in fetch_chunk (state=<optimized out>)
    at platform/x11/curl_stream.c:287
#9  fetcher_thread (state=<optimized out>) at platform/x11/curl_stream.c:318
#10 pthread_thread (arg=0xdc8fb4de000) at platform/x11/curl_stream.c:102
#11 0x00000dc8fed8477e in _rthread_start (v=0x1) at /usr/src/lib/librthread/rthread.c:145
#12 0x00000dc8f933252b in __tfork_thread ()
    at /usr/src/lib/libc/arch/amd64/sys/tfork_thread.S:75
#13 0x0000000000000000 in ?? ()
(gdb) frame 1
#1  0x00000dc852af5bbb in _xcb_socket (family=1, type=1, proto=0)
    at /usr/xenocara/lib/libxcb/libxcb/../../../dist/libxcb/src/xcb_util.c:315
315    fd = socket(family, type | SOCK_CLOEXEC, proto);
(gdb) frame 7
#7  0x00000dc60370f4e0 in winreloadpage (app=<optimized out>)
    at platform/x11/x11_main.c:701
701 Display *dpy = XOpenDisplay(NULL);

mupdf-x11-curl(1655): syscall 97 "unix"    -- socket
mupdf-x11-curl(12083): syscall 48 "stdio"  -- sigprocmask

Reply | Threaded
Open this post in threaded view
|

Re: pledge in ports: textproc/mupdf

Sebastien Marie-2
On Mon, Jan 18, 2016 at 01:50:01PM +0000, Stuart Henderson wrote:

>
> These seem fine in my tests so far (tried a few files with various
> image formats etc and some with passwords).
>
> >   - mupdf-x11-curl
>
> I ran into a problem with mupdf-x11-curl fetching from http getting
> killed on the first file I tried: as luck would have it I haven't been
> able to repeat it with another file, but http://dmtx.uk/ugh.pdf
> if you'd like to try it. Works ok from disk. It fails to display
> in the unpledged version too (sits burning cpu), so there's an
> upstream bug anyway.
>

I am able to reproduce it.

The problem is in mupdf-1.8-source/platform/x11/x11_main.c (but
mupdf-x11 isn't impacted).

The function winreloadpage() calls XOpenDisplay() in the middle of the
program (whereas xdpy contains the already opened display ? I dunno why
it tries to open it again).

void winreloadpage(pdfapp_t *app)
{
        XEvent xev;
        Display *dpy = XOpenDisplay(NULL);

But as we tries to open display, it is depending of DISPLAY env for how
xcb will contact the Xserver. In your backtrace (and mine) it is :0, so
it tries unix sockets... and pledge don't let it to pass.

This function seems to be called only from HAVE_CURL code (it is why
mupdf-x11 isn't impacted), and seems to be used to "emulate" a reload
command for http (as the pdf stream arrived by chunks). It downloads a
block of octets, try to display a page (reload), if fails downloads more
octets... and loop.

The following diff adds removing of the extra XOpenDisplay() and reuse
xdpy variable. It makes your uri to have same behaviour than unpledged
version... (still bugged).

--
Sebastien Marie

Index: Makefile
===================================================================
RCS file: /cvs/ports/textproc/mupdf/Makefile,v
retrieving revision 1.59
diff -u -p -r1.59 Makefile
--- Makefile 12 Nov 2015 17:26:54 -0000 1.59
+++ Makefile 18 Jan 2016 14:23:32 -0000
@@ -8,6 +8,7 @@ COMMENT = graphic library, pdf parser, v
 V = 1.8
 DISTNAME = mupdf-$V-source
 PKGNAME = mupdf-$V
+REVISION = 0
 
 CATEGORIES = textproc x11
 
@@ -23,6 +24,7 @@ PERMIT_PACKAGE_CDROM = Yes
 FLAVORS= js
 FLAVOR?=
 
+# uses pledge(2)
 WANTLIB += GL X11 Xcursor Xext Xinerama Xrandr c crypto curl freetype
 WANTLIB += idn jbig2dec jpeg m nghttp2 openjp2 pthread ssl z
 
Index: patches/patch-platform_gl_gl-main_c
===================================================================
RCS file: patches/patch-platform_gl_gl-main_c
diff -N patches/patch-platform_gl_gl-main_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-platform_gl_gl-main_c 18 Jan 2016 14:23:32 -0000
@@ -0,0 +1,21 @@
+$OpenBSD$
+
+add pledge(2) to mupdf-gl:
+  - drm : opengl stuff
+  - proc exec : opening a external browser on uri link
+
+--- platform/gl/gl-main.c.orig Tue Nov 10 17:19:51 2015
++++ platform/gl/gl-main.c Sun Jan 17 10:21:44 2016
+@@ -1361,6 +1361,12 @@ int main(int argc, char **argv)
+
+ glfwMakeContextCurrent(window);
+
++ if (pledge("stdio rpath drm proc exec", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++
+ ctx = fz_new_context(NULL, NULL, 0);
+ fz_register_document_handlers(ctx);
+
Index: patches/patch-platform_x11_jstest_main_c
===================================================================
RCS file: patches/patch-platform_x11_jstest_main_c
diff -N patches/patch-platform_x11_jstest_main_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-platform_x11_jstest_main_c 18 Jan 2016 14:23:32 -0000
@@ -0,0 +1,21 @@
+$OpenBSD$
+
+add pledge(2) to mujstest
+  - rpath : OPEN command
+  - wpath cpath : SCREENSHOT command
+
+--- platform/x11/jstest_main.c.orig Sun Jan 17 13:50:58 2016
++++ platform/x11/jstest_main.c Sun Jan 17 13:52:29 2016
+@@ -310,6 +310,12 @@ main(int argc, char *argv[])
+ if (fz_optind == argc)
+ usage();
+
++ if (pledge("stdio rpath wpath cpath", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++
+ ctx = fz_new_context(NULL, NULL, FZ_STORE_DEFAULT);
+ if (!ctx)
+ {
Index: patches/patch-platform_x11_x11_main_c
===================================================================
RCS file: patches/patch-platform_x11_x11_main_c
diff -N patches/patch-platform_x11_x11_main_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-platform_x11_x11_main_c 18 Jan 2016 14:23:32 -0000
@@ -0,0 +1,47 @@
+$OpenBSD$
+
+add pledge(2) to mupdf-x11 and mupdf-x11-curl:
+  - inet dns : with HAVE_CURL only
+  - proc exec : opening an external browser on uri link
+
+--- platform/x11/x11_main.c.orig Tue Nov 10 17:19:51 2015
++++ platform/x11/x11_main.c Mon Jan 18 15:20:56 2016
+@@ -698,7 +698,6 @@ void onselreq(Window requestor, Atom selection, Atom t
+ void winreloadpage(pdfapp_t *app)
+ {
+ XEvent xev;
+- Display *dpy = XOpenDisplay(NULL);
+
+ xev.xclient.type = ClientMessage;
+ xev.xclient.serial = 0;
+@@ -709,8 +708,7 @@ void winreloadpage(pdfapp_t *app)
+ xev.xclient.data.l[0] = 0;
+ xev.xclient.data.l[1] = 0;
+ xev.xclient.data.l[2] = 0;
+- XSendEvent(dpy, xwin, 0, 0, &xev);
+- XCloseDisplay(dpy);
++ XSendEvent(xdpy, xwin, 0, 0, &xev);
+ }
+
+ void winopenuri(pdfapp_t *app, char *buf)
+@@ -884,6 +882,20 @@ int main(int argc, char **argv)
+ tmo_at.tv_sec = 0;
+ tmo_at.tv_usec = 0;
+ timeout = NULL;
++
++#ifdef HAVE_CURL
++ if (pledge("stdio rpath inet dns proc exec", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++#else
++ if (pledge("stdio rpath proc exec", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++#endif
+
+ pdfapp_open(&gapp, filename, 0);
+
Index: patches/patch-source_tools_mudraw_c
===================================================================
RCS file: patches/patch-source_tools_mudraw_c
diff -N patches/patch-source_tools_mudraw_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-source_tools_mudraw_c 18 Jan 2016 14:23:32 -0000
@@ -0,0 +1,31 @@
+$OpenBSD$
+
+add pledge(2) to "mutool draw":
+  - wpath cpath : only if output is specified
+
+--- source/tools/mudraw.c.orig Tue Nov 10 17:19:51 2015
++++ source/tools/mudraw.c Sun Jan 17 10:21:04 2016
+@@ -909,6 +909,23 @@ int mudraw_main(int argc, char **argv)
+ if (fz_optind == argc)
+ usage();
+
++ if (output && output[0] != '-' && *output != 0)
++ {
++ if (pledge("stdio rpath wpath cpath", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++ }
++ else
++ {
++ if (pledge("stdio rpath", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++ }
++
+ ctx = fz_new_context((showmemory == 0 ? NULL : &alloc_ctx), NULL, FZ_STORE_DEFAULT);
+ if (!ctx)
+ {
Index: patches/patch-source_tools_pdfclean_c
===================================================================
RCS file: patches/patch-source_tools_pdfclean_c
diff -N patches/patch-source_tools_pdfclean_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-source_tools_pdfclean_c 18 Jan 2016 14:23:32 -0000
@@ -0,0 +1,19 @@
+$OpenBSD$
+
+add pledge(2) to "mutool clean"
+
+--- source/tools/pdfclean.c.orig Sun Jan 17 11:59:42 2016
++++ source/tools/pdfclean.c Sun Jan 17 12:01:42 2016
+@@ -79,6 +79,12 @@ int pdfclean_main(int argc, char **argv)
+ outfile = argv[fz_optind++];
+ }
+
++ if (pledge("stdio rpath wpath cpath", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++
+ ctx = fz_new_context(NULL, NULL, FZ_STORE_UNLIMITED);
+ if (!ctx)
+ {
Index: patches/patch-source_tools_pdfextract_c
===================================================================
RCS file: patches/patch-source_tools_pdfextract_c
diff -N patches/patch-source_tools_pdfextract_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-source_tools_pdfextract_c 18 Jan 2016 14:23:32 -0000
@@ -0,0 +1,19 @@
+$OpenBSD$
+
+add pledge(2) to "mutool extract"
+
+--- source/tools/pdfextract.c.orig Sun Jan 17 12:12:27 2016
++++ source/tools/pdfextract.c Sun Jan 17 12:13:42 2016
+@@ -202,6 +202,12 @@ int pdfextract_main(int argc, char **argv)
+
+ infile = argv[fz_optind++];
+
++ if (pledge("stdio rpath wpath cpath", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++
+ ctx = fz_new_context(NULL, NULL, FZ_STORE_UNLIMITED);
+ if (!ctx)
+ {
Index: patches/patch-source_tools_pdfinfo_c
===================================================================
RCS file: patches/patch-source_tools_pdfinfo_c
diff -N patches/patch-source_tools_pdfinfo_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-source_tools_pdfinfo_c 18 Jan 2016 14:23:32 -0000
@@ -0,0 +1,19 @@
+$OpenBSD$
+
+add pledge(2) to "mutool info"
+
+--- source/tools/pdfinfo.c.orig Sun Jan 17 10:20:26 2016
++++ source/tools/pdfinfo.c Sun Jan 17 10:23:26 2016
+@@ -1061,6 +1061,12 @@ int pdfinfo_main(int argc, char **argv)
+ if (fz_optind == argc)
+ infousage();
+
++ if (pledge("stdio rpath", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++
+ ctx = fz_new_context(NULL, NULL, FZ_STORE_UNLIMITED);
+ if (!ctx)
+ {
Index: patches/patch-source_tools_pdfpages_c
===================================================================
RCS file: patches/patch-source_tools_pdfpages_c
diff -N patches/patch-source_tools_pdfpages_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-source_tools_pdfpages_c 18 Jan 2016 14:23:32 -0000
@@ -0,0 +1,19 @@
+$OpenBSD$
+
+add pledge(2) to "mutool pages"
+
+--- source/tools/pdfpages.c.orig Sun Jan 17 12:13:56 2016
++++ source/tools/pdfpages.c Sun Jan 17 12:15:06 2016
+@@ -229,6 +229,12 @@ int pdfpages_main(int argc, char **argv)
+ if (fz_optind == argc)
+ infousage();
+
++ if (pledge("stdio rpath", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++
+ ctx = fz_new_context(NULL, NULL, FZ_STORE_UNLIMITED);
+ if (!ctx)
+ {
Index: patches/patch-source_tools_pdfposter_c
===================================================================
RCS file: patches/patch-source_tools_pdfposter_c
diff -N patches/patch-source_tools_pdfposter_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-source_tools_pdfposter_c 18 Jan 2016 14:23:32 -0000
@@ -0,0 +1,19 @@
+$OpenBSD$
+
+add pledge(2) to "mutool poster"
+
+--- source/tools/pdfposter.c.orig Sun Jan 17 12:21:27 2016
++++ source/tools/pdfposter.c Sun Jan 17 12:22:56 2016
+@@ -189,6 +189,12 @@ int pdfposter_main(int argc, char **argv)
+ outfile = argv[fz_optind++];
+ }
+
++ if (pledge("stdio rpath wpath cpath", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++
+ ctx = fz_new_context(NULL, NULL, FZ_STORE_UNLIMITED);
+ if (!ctx)
+ {
Index: patches/patch-source_tools_pdfshow_c
===================================================================
RCS file: patches/patch-source_tools_pdfshow_c
diff -N patches/patch-source_tools_pdfshow_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-source_tools_pdfshow_c 18 Jan 2016 14:23:32 -0000
@@ -0,0 +1,35 @@
+$OpenBSD$
+
+add pledge(2) to "mutool show":
+  - rpath : dropped after opening
+  - no need of wpath cpath for -o, as the open is already done at this place
+
+--- source/tools/pdfshow.c.orig Sun Jan 17 09:08:52 2016
++++ source/tools/pdfshow.c Sun Jan 17 09:15:05 2016
+@@ -247,6 +247,12 @@ int pdfshow_main(int argc, char **argv)
+ }
+ }
+
++ if (pledge("stdio rpath", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++
+ ctx = fz_new_context(NULL, NULL, FZ_STORE_UNLIMITED);
+ if (!ctx)
+ {
+@@ -258,6 +264,13 @@ int pdfshow_main(int argc, char **argv)
+ fz_try(ctx)
+ {
+ doc = pdf_open_document(ctx, filename);
++
++ if (pledge("stdio", NULL) == -1)
++ {
++ fprintf(stderr, "pledge: %s\n", strerror(errno));
++ exit(1);
++ }
++
+ if (pdf_needs_password(ctx, doc))
+ if (!pdf_authenticate_password(ctx, doc, password))
+ fz_warn(ctx, "cannot authenticate password: %s", filename);

12