On Sun, Feb 16, 2025 at 09:44:50PM +0100, Matthieu Herrb wrote:
> On Sun, Dec 15, 2024 at 07:08:23PM +0100, Matthieu Herrb wrote:

> > So mplayer is using the XShm extention to share memory with the X
> > server. Unfortunatly, on OpenBSD the X server and the mplayer process
> > don't run with the same uid, leading to the patches to set the shared
> > memory segement permissions to 0777. (one can see them with `ipcs -m`
> > while mplayer is running).

> > A number of years ago the X.Org developpers provided a solution for
> > this in XShm 1.2. Unfortunatly at this time they only implemented it
> > in libxcb, considering that it would soon replace libX11 and libXext
> > completely, and this has prevented many projects using XShm to upgrade
> > their code.

> > Here is a relarively crude patch to x11/mplayer to implement XShm 1.2
> > support in vo_x11 and vo_xv backends. The configure bits in particular
> > are not  very nice and will probably require more work to be
> > upstreamed.

> > But at least there are no longer wide open shared memory segments
> > containing the p0rn you're watching while running mplayer.

> > ok, comments ?

> Ping. Any interest in getting mplayer a bit more secure ?

I've used this and have been using mplayer for a few hours now with
no issues. I say go for it.

ok kmos

--Kurt

> Index: Makefile
> ===================================================================
> RCS file: /local/cvs/ports/x11/mplayer/Makefile,v
> diff -u -p -u -r1.333 Makefile
> --- Makefile  2 Dec 2024 06:14:53 -0000       1.333
> +++ Makefile  16 Feb 2025 20:43:00 -0000
> @@ -3,7 +3,7 @@ COMMENT=      movie player supporting many fo
>  V=           20240803
>  FFMPEG_V=    6.1.2
>  DISTNAME=    mplayer-${V}
> -REVISION=    1
> +REVISION=    2
>  CATEGORIES=  x11 multimedia
>  SITES=               https://comstyle.com/source/
>  EXTRACT_SUFX=        .tar.xz
> @@ -17,12 +17,12 @@ MAINTAINER=       Brad Smith <b...@comstyle.co
>  # GPLv2
>  PERMIT_PACKAGE=      Yes
>  
> -WANTLIB=     EGL GL SDL X11 Xext Xinerama Xss Xv Xxf86dga Xxf86vm ass \
> +WANTLIB=     EGL GL SDL X11 X11-xcb Xext Xinerama Xss Xv Xxf86dga Xxf86vm 
> ass \
>               avcodec avformat avutil bluray bs2b bz2 c cdda_interface \
>               cdda_paranoia crypto dv dvdnav dvdread enca fontconfig \
>               freetype fribidi gif iconv jpeg m mng mp3lame curses png \
>               postproc pthread sndio ssl swscale swresample util x264 \
> -             xvidcore z
> +             xcb xcb-shm xvidcore z
>  
>  COMPILER=    base-clang ports-gcc
>  COMPILER_LANGS=      c
> Index: patches/patch-configure
> ===================================================================
> RCS file: /local/cvs/ports/x11/mplayer/patches/patch-configure,v
> diff -u -p -u -r1.97 patch-configure
> --- patches/patch-configure   24 Nov 2024 08:50:45 -0000      1.97
> +++ patches/patch-configure   16 Feb 2025 20:43:00 -0000
> @@ -1,3 +1,5 @@
> +Add support for xcb-shm
> +
>  Index: configure
>  --- configure.orig
>  +++ configure
> @@ -15,7 +17,24 @@ Index: configure
>   # Use this before starting a check
>   echocheck() {
>     echo "============ Checking for $@ ============" >> "$TMPLOG"
> -@@ -1521,39 +1528,39 @@ echo configuration: $configuration > "$TMPLOG"
> +@@ -694,6 +701,7 @@ _mplayer=yes
> + _x11=auto
> + _xshape=auto
> + _xss=auto
> ++_xcb_shm=auto
> + _dga1=auto
> + _dga2=auto
> + _xv=auto
> +@@ -1042,6 +1050,8 @@ for ac_option do
> +   --disable-xshape)     _xshape=no      ;;
> +   --enable-xss)         _xss=yes        ;;
> +   --disable-xss)        _xss=no         ;;
> ++  --enable-xcb_shm)     _xcb_shm=yes    ;;
> ++  --disable-xcb-shm)    _xcb_shm=no     ;;  
> +   --enable-xv)          _xv=yes         ;;
> +   --disable-xv)         _xv=no          ;;
> +   --enable-vda)         _vda=yes        ;;
> +@@ -1521,39 +1531,39 @@ echo configuration: $configuration > "$TMPLOG"
>   echo >> "$TMPLOG"
>   
>   
> @@ -87,7 +106,7 @@ Index: configure
>   list_subparts() {
>     test ! -e ffmpeg/libav${3} && return 1
>     pattern="s/^[^#]*${1}.*([^ ,]*, *\([^ ,)]*\).*/\1_${2}/p"
> -@@ -1990,7 +1997,10 @@ fi
> +@@ -1990,7 +2000,10 @@ fi
>   if arm ; then
>     cc_check && host_arch=aarch64 || host_arch=arm
>   fi
> @@ -98,7 +117,7 @@ Index: configure
>   
>   echo "Detected operating system: $system_name"
>   echo "Detected host architecture: $host_arch"
> -@@ -2544,7 +2554,7 @@ case "$host_arch" in
> +@@ -2544,7 +2557,7 @@ case "$host_arch" in
>       arch='sparc'
>       iproc='sparc'
>       if test "$host_arch" = "sparc64" ; then
> @@ -107,7 +126,7 @@ Index: configure
>         proc='ultrasparc'
>         def_fast_64bit='#define HAVE_FAST_64BIT 1'
>       elif sunos ; then
> -@@ -2846,7 +2856,14 @@ EOF
> +@@ -2846,7 +2859,14 @@ EOF
>       arch='arc'
>       iproc='arc'
>       ;;
> @@ -122,7 +141,7 @@ Index: configure
>     *)
>       echo "The architecture of your CPU ($host_arch) is not supported by 
> this configure script"
>       echo "It seems nobody has ported MPlayer to your OS or CPU type yet."
> -@@ -2920,7 +2937,7 @@ cat > $TMPC << EOF
> +@@ -2920,7 +2940,7 @@ cat > $TMPC << EOF
>   int ff_extern;
>   EOF
>   cc_check -c || die "Symbol mangling check failed."
> @@ -131,7 +150,7 @@ Index: configure
>   extern_prefix=${sym%%ff_extern*}
>   def_extern_asm="#define EXTERN_ASM $extern_prefix"
>   def_extern_prefix="#define EXTERN_PREFIX \"$extern_prefix\""
> -@@ -2985,7 +3002,7 @@ else
> +@@ -2985,7 +3005,7 @@ else
>   fi
>   
>   CFLAGS="-D_ISOC99_SOURCE -I. -Iffmpeg $CFLAGS"
> @@ -140,7 +159,7 @@ Index: configure
>   
>   # On glibc, add some more CPPFLAGS for enabling required functionality.
>   cpp_condition_check features.h "defined __GLIBC__" &&
> -@@ -3066,7 +3083,7 @@ elif test $relocatable = "yes" ; then
> +@@ -3066,7 +3086,7 @@ elif test $relocatable = "yes" ; then
>   fi
>   echores $relocatable
>   
> @@ -149,7 +168,27 @@ Index: configure
>     # Checking assembler (_as) compatibility...
>     # Added workaround for older as that reads from stdin by default - atmos
>     as_version=$(echo '' | $_as -version 2>&1 | sed -n 's/^.*assembler 
> \(version \)*\([0-9.]*\).*$/\2/p')
> -@@ -6338,12 +6355,16 @@ fi #if irix
> +@@ -4982,6 +5002,19 @@ else
> + fi
> + echores "$_x11"
> + 
> ++echocheck "xcb shm extension"
> ++if test "$_xcb_shm" = auto ; then
> ++  _xcb_shm=no
> ++  statement_check "xcb/shm.h" 'xcb_shm_query_version(NULL)' -lX11-xcb 
> -lxcb-shm -lxcb && _xcb_shm=yes
> ++fi
> ++if test "$_xcb_shm" = yes ; then
> ++  def_xcb_shm='#define CONFIG_XCB_SHM 1'
> ++  libs_mplayer="$libs_mplayer -lX11-xcb -lxcb-shm -lxcb"
> ++else
> ++  def_xcb_shm='#undef CONFIG_XCB_SHM'
> ++fi
> ++echores "$_xcb_shm"
> ++
> + echocheck "Xss screensaver extensions"
> + if test "$_xss" = auto ; then
> +   _xss=no
> +@@ -6338,12 +6371,16 @@ fi #if irix
>   echocheck "sndio audio"
>   if test "$_sndio" = auto ; then
>     _sndio=no
> @@ -168,7 +207,7 @@ Index: configure
>   else
>     def_sndio='#undef CONFIG_SNDIO_AUDIO'
>     noaomodules="sndio $noaomodules"
> -@@ -6526,7 +6547,7 @@ echocheck "cdparanoia"
> +@@ -6526,7 +6563,7 @@ echocheck "cdparanoia"
>   if test "$_cdparanoia" = auto ; then
>       _cdparanoia=no
>       for inc_tmp in "" "-I/usr/include/cdda" "-I/usr/local/include/cdda" ; do
> @@ -177,7 +216,7 @@ Index: configure
>           _cdparanoia=yes && extra_cflags="$extra_cflags $inc_tmp" && break
>       done
>   fi
> -@@ -8417,6 +8438,7 @@ extra_ldflags="$extra_ldflags $libm"
> +@@ -8417,6 +8454,7 @@ extra_ldflags="$extra_ldflags $libm"
>   # XML documentation tests
>   echocheck "XML catalogs"
>   for try_catalog in \
> @@ -185,7 +224,7 @@ Index: configure
>     /etc/sgml/catalog \
>     /usr/share/xml/docbook/*/catalog.xml \
>     /opt/local/share/xml/docbook-xml/*/catalog.xml \
> -@@ -8444,6 +8466,7 @@ fi
> +@@ -8444,6 +8482,7 @@ fi
>   
>   echocheck "XML chunked stylesheet"
>   for try_chunk_xsl in \
> @@ -193,7 +232,7 @@ Index: configure
>     /usr/share/xml/docbook/*/html/chunk.xsl \
>     /usr/share/sgml/docbook/stylesheet/xsl/nwalsh/html/chunk.xsl \
>     /usr/share/sgml/docbook/yelp/docbook/html/chunk.xsl \
> -@@ -8469,6 +8492,7 @@ fi
> +@@ -8469,6 +8508,7 @@ fi
>   
>   echocheck "XML monolithic stylesheet"
>   for try_docbook_xsl in \
> @@ -201,7 +240,7 @@ Index: configure
>     /usr/share/xml/docbook/*/html/docbook.xsl \
>     /usr/share/sgml/docbook/stylesheet/xsl/nwalsh/html/docbook.xsl \
>     /usr/share/sgml/docbook/yelp/docbook/html/docbook.xsl \
> -@@ -8522,6 +8546,7 @@ EOF
> +@@ -8522,6 +8562,7 @@ EOF
>   echocheck "XML DTD"
>   #FIXME: This should prefer higher version numbers, not the other way around 
> ..
>   for try_dtd in \
> @@ -209,7 +248,15 @@ Index: configure
>     /usr/share/xml/docbook/*/dtd/4*/docbookx.dtd \
>     /usr/share/xml/docbook/*/docbookx.dtd \
>     /usr/share/sgml/docbook/*/docbookx.dtd \
> -@@ -9732,9 +9757,6 @@ cmp -s "$TMPH" config.h || mv -f "$TMPH" config.h
> +@@ -9433,6 +9474,7 @@ $def_xf86keysym
> + $def_xinerama
> + $def_xmga
> + $def_xss
> ++$def_xcb_shm
> + $def_xv
> + $def_xvr100
> + $def_yuv4mpeg
> +@@ -9732,9 +9774,6 @@ cmp -s "$TMPH" config.h || mv -f "$TMPH" config.h
>   
>   ############################################################################
>   
> @@ -219,7 +266,7 @@ Index: configure
>   # Create avconfig.h for FFmpeg.
>   cat > "$TMPH" << EOF
>   /* Generated by mpconfigure */
> -@@ -9827,8 +9849,6 @@ print_enabled_components libavformat/demuxer_list.c AV
> +@@ -9827,8 +9866,6 @@ print_enabled_components libavformat/demuxer_list.c AV
>   print_enabled_components libavformat/muxer_list.c AVOutputFormat muxer_list 
> $libavmuxers
>   print_enabled_components libavformat/protocol_list.c URLProtocol 
> url_protocols $libavprotocols
>   print_enabled_filters libavfilter/filter_list.c AVFilter filter_list 
> $libavfilters
> Index: patches/patch-libvo_vo_x11_c
> ===================================================================
> RCS file: /local/cvs/ports/x11/mplayer/patches/patch-libvo_vo_x11_c,v
> diff -u -p -u -r1.1 patch-libvo_vo_x11_c
> --- patches/patch-libvo_vo_x11_c      2 Dec 2024 06:14:53 -0000       1.1
> +++ patches/patch-libvo_vo_x11_c      16 Feb 2025 20:43:00 -0000
> @@ -1,15 +1,117 @@
> -Revert "Use appropriate shared memory permissions."
> -r38419
> +Implement XShm 1.2
>  
>  Index: libvo/vo_x11.c
>  --- libvo/vo_x11.c.orig
>  +++ libvo/vo_x11.c
> -@@ -150,7 +150,7 @@ static void getMyXImage(void)
> +@@ -38,7 +38,11 @@
> + #ifdef HAVE_SHM
> + #include <sys/ipc.h>
> + #include <sys/shm.h>
> ++#include <sys/mman.h>
> + #include <X11/extensions/XShm.h>
> ++#include <X11/Xlib-xcb.h>
> ++#include <xcb/shm.h>
> ++#include <unistd.h>
> + 
> + static int Shmem_Flag;
> + 
> +@@ -78,7 +82,9 @@ static unsigned char *ImageDataOrig;
> + static XImage *myximage = NULL;
> + static int depth, bpp;
> + static XWindowAttributes attribs;
> +-
> ++#ifdef HAVE_SHM
> ++static  char myshmname[128];
> ++#endif
> + static int int_pause;
> + 
> + static int Flip_Flag;
> +@@ -123,9 +129,23 @@ static int dst_width;
> + 
> + static XVisualInfo vinfo;
> + 
> ++#ifdef HAVE_SHM
> ++static Bool XShmAttachFd(Display *dpy, XShmSegmentInfo *shminfo)
> ++{
> ++    xcb_connection_t *xcb_conn = XGetXCBConnection(dpy);
> ++        
> ++    shminfo->shmseg = xcb_generate_id(xcb_conn);
> ++    xcb_shm_attach_fd(xcb_conn, shminfo->shmseg,
> ++                      shminfo->shmid, shminfo->readOnly);
> ++    return 1;
> ++}
> ++#endif
> ++
> + static void getMyXImage(void)
> + {
> + #ifdef HAVE_SHM
> ++    size_t len;
> ++
> +     if (mLocalDisplay && XShmQueryExtension(mDisplay))
> +         Shmem_Flag = 1;
> +     else
> +@@ -148,33 +168,40 @@ static void getMyXImage(void)
> +                    "Shared memory error,disabling ( Ximage error )\n");
> +             goto shmemerror;
>           }
> -         Shminfo[0].shmid = shmget(IPC_PRIVATE,
> -                                   myximage->bytes_per_line *
> +-        Shminfo[0].shmid = shmget(IPC_PRIVATE,
> +-                                  myximage->bytes_per_line *
>  -                                  myximage->height, IPC_CREAT | SHM_R | 
> SHM_W);
> -+                                  myximage->height, IPC_CREAT | 0777);
> -         if (Shminfo[0].shmid < 0)
> +-        if (Shminfo[0].shmid < 0)
> ++    memcpy(myshmname, "/tmp/mplayer-x11-XXXXXXXXXX", sizeof(myshmname));
> ++    Shminfo[0].shmid = shm_mkstemp(myshmname);
> ++    if (Shminfo[0].shmid < 0)
>           {
>               XDestroyImage(myximage);
> +             mp_msg(MSGT_VO, MSGL_V, "%s\n", strerror(errno));
> +             //perror( strerror( errno ) );
> +             mp_msg(MSGT_VO, MSGL_WARN,
> +-                   "Shared memory error,disabling ( seg id error )\n");
> ++                   "Shared memory error,disabling ( shm_open error )\n");
> +             goto shmemerror;
> +         }
> +-        Shminfo[0].shmaddr = (char *) shmat(Shminfo[0].shmid, 0, 0);
> +-
> +-        if (Shminfo[0].shmaddr == ((char *) -1))
> ++    len = myximage->bytes_per_line * myximage->height;
> ++    
> ++        Shminfo[0].shmaddr = mmap(NULL, len, PROT_READ | PROT_WRITE,
> ++            MAP_SHARED|__MAP_NOFAULT, Shminfo[0].shmid, 0);
> ++    
> ++        if (Shminfo[0].shmaddr == MAP_FAILED)
> +         {
> +             XDestroyImage(myximage);
> +-            if (Shminfo[0].shmaddr != ((char *) -1))
> +-                shmdt(Shminfo[0].shmaddr);
> +             mp_msg(MSGT_VO, MSGL_WARN,
> +-                   "Shared memory error,disabling ( address error )\n");
> ++                   "Shared memory error,disabling ( mmap error )\n");
> +             goto shmemerror;
> +         }
> ++    if (ftruncate(Shminfo[0].shmid, len) == -1)
> ++    {
> ++            XDestroyImage(myximage);
> ++            mp_msg(MSGT_VO, MSGL_WARN,
> ++                   "Shared memory error,disabling ( fruncate error )\n");
> ++            goto shmemerror;
> ++    }
> +         myximage->data = Shminfo[0].shmaddr;
> +         ImageData = (unsigned char *) myximage->data;
> +         Shminfo[0].readOnly = False;
> +-        XShmAttach(mDisplay, &Shminfo[0]);
> ++        XShmAttachFd(mDisplay, &Shminfo[0]);
> + 
> +         XSync(mDisplay, False);
> + 
> +@@ -218,9 +245,10 @@ static void freeMyXImage(void)
> + #ifdef HAVE_SHM
> +     if (Shmem_Flag)
> +     {
> ++    close(Shminfo[0].shmid);
> ++    shm_unlink(myshmname);
> +         XShmDetach(mDisplay, &Shminfo[0]);
> +         XDestroyImage(myximage);
> +-        shmdt(Shminfo[0].shmaddr);
> +     } else
> + #endif
> +     {
> Index: patches/patch-libvo_vo_xv_c
> ===================================================================
> RCS file: /local/cvs/ports/x11/mplayer/patches/patch-libvo_vo_xv_c,v
> diff -u -p -u -r1.1 patch-libvo_vo_xv_c
> --- patches/patch-libvo_vo_xv_c       2 Dec 2024 06:14:53 -0000       1.1
> +++ patches/patch-libvo_vo_xv_c       16 Feb 2025 20:43:00 -0000
> @@ -1,15 +1,78 @@
> -Revert "Use appropriate shared memory permissions."
> -r38419
> +Implement XShm 1.2
>  
>  Index: libvo/vo_xv.c
>  --- libvo/vo_xv.c.orig
>  +++ libvo/vo_xv.c
> -@@ -279,7 +279,7 @@ static void allocate_xvimage(int foo)
> +@@ -74,7 +74,11 @@ const LIBVO_EXTERN(xv)
> + #ifdef HAVE_SHM
> + #include <sys/ipc.h>
> + #include <sys/shm.h>
> ++#include <sys/mman.h>
> + #include <X11/extensions/XShm.h>
> ++#include <X11/Xlib-xcb.h>
> ++#include <xcb/shm.h>
> ++#include <unistd.h>
> + 
> + static XShmSegmentInfo Shminfo[NUM_BUFFERS];
> + static int Shmem_Flag;
> +@@ -97,6 +101,9 @@ static int num_buffers = 1;     // default
> + static int visible_buf = -1;    // -1 means: no buffer was drawn yet
> + static XvImage *xvimage[NUM_BUFFERS];
> + 
> ++#ifdef HAVE_SHM
> ++static  char myshmname[128];
> ++#endif
> + 
> + static uint32_t image_width;
> + static uint32_t image_height;
> +@@ -110,6 +117,18 @@ static uint32_t max_width = 0, max_height = 0; // zero
> + 
> + static vo_draw_alpha_func draw_alpha_func;
> + 
> ++#ifdef HAVE_SHM
> ++static Bool XShmAttachFd(Display *dpy, XShmSegmentInfo *shminfo)
> ++{
> ++    xcb_connection_t *xcb_conn = XGetXCBConnection(dpy);
> ++        
> ++    shminfo->shmseg = xcb_generate_id(xcb_conn);
> ++    xcb_shm_attach_fd(xcb_conn, shminfo->shmseg,
> ++                      shminfo->shmid, shminfo->readOnly);
> ++    return 1;
> ++}
> ++#endif
> ++
> + static void fixup_osd_position(int *x0, int *y0, int *w, int *h)
> + {
> +     *x0 += image_width * (vo_panscan_x >> 1) / (vo_dwidth + vo_panscan_x);
> +@@ -278,15 +297,18 @@ static void allocate_xvimage(int foo)
> +                                          NULL, image_width, image_height,
>                                            &Shminfo[foo]);
>   
> -         Shminfo[foo].shmid =
> +-        Shminfo[foo].shmid =
>  -            shmget(IPC_PRIVATE, xvimage[foo]->data_size, IPC_CREAT | SHM_R 
> | SHM_W);
> -+            shmget(IPC_PRIVATE, xvimage[foo]->data_size, IPC_CREAT | 0777);
> -         Shminfo[foo].shmaddr = (char *) shmat(Shminfo[foo].shmid, 0, 0);
> +-        Shminfo[foo].shmaddr = (char *) shmat(Shminfo[foo].shmid, 0, 0);
> ++    memcpy(myshmname, "/tmp/mplayer-xv-XXXXXXXXXX", sizeof(myshmname));
> ++    Shminfo[foo].shmid = shm_mkstemp(myshmname);
> ++        Shminfo[foo].shmaddr = mmap(NULL, xvimage[foo]->data_size,
> ++        PROT_READ | PROT_WRITE, MAP_SHARED|__MAP_NOFAULT,
> ++        Shminfo[foo].shmid, 0);
> ++    ftruncate(Shminfo[foo].shmid, xvimage[foo]->data_size);
>           Shminfo[foo].readOnly = False;
>   
> +         xvimage[foo]->data = Shminfo[foo].shmaddr;
> +-        XShmAttach(mDisplay, &Shminfo[foo]);
> ++        XShmAttachFd(mDisplay, &Shminfo[foo]);
> +         XSync(mDisplay, False);
> +-        shmctl(Shminfo[foo].shmid, IPC_RMID, 0);
> ++    shm_unlink(myshmname);
> +     } else
> + #endif
> +     {
> +@@ -306,6 +328,7 @@ static void deallocate_xvimage(int foo)
> +     if (Shmem_Flag)
> +     {
> +         XShmDetach(mDisplay, &Shminfo[foo]);
> ++    close(Shminfo[foo].shmid);
> +         shmdt(Shminfo[foo].shmaddr);
> +     } else
> + #endif
> 
> -- 
> Matthieu Herrb
> 

Reply via email to