| Message ID | 1440420170-13337-4-git-send-email-patrik.jakobsson@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Aug 24, 2015 at 02:42:48PM +0200, Patrik Jakobsson wrote: > * Makefile.am: Add compilation of drm.c. > * defs.h: Add extern declaration of drm_ioctl when drm headers are found. > * drm.c: New file. > * ioctl.c (ioctl_decode): Dispatch drm ioctls when drm headers are found. * defs.h (drm_decode_number, drm_ioctl): New prototypes. * drm.c: New file. * Makefile.am (strace_SOURCES): Add it. * ioctl.c (ioctl_decode_command_number, ioctl_decode) [HAVE_DRM_H || HAVE_DRM_DRM_H]: Dispatch drm ioctls. > --- a/defs.h > +++ b/defs.h > @@ -612,6 +612,9 @@ extern const char *sprint_open_modes(int); > extern void print_seccomp_filter(struct tcb *tcp, unsigned long); > > extern int block_ioctl(struct tcb *, const unsigned int, long); > +#if defined(HAVE_DRM_H) || defined(HAVE_DRM_DRM_H) > +extern int drm_ioctl(struct tcb *, const unsigned int, long); > +#endif I think function these prototypes could be added unconditionally. Note that v3 version of this patch also declared drm_decode_number(). > --- /dev/null > +++ b/drm.c [...] > +#include "defs.h" > + > +#if defined(HAVE_DRM_H) || defined(HAVE_DRM_DRM_H) > + > +#ifdef HAVE_DRM_H > +#include <drm.h> > +#else > +#include <drm/drm.h> > +#endif > + > +#include <sys/param.h> Let's include <sys/param.h> before drm stuff. > +#define DRM_MAX_NAME_LEN 128 > + > +inline int drm_is_priv(const unsigned int num) > +{ > + return (_IOC_NR(num) >= DRM_COMMAND_BASE && > + _IOC_NR(num) < DRM_COMMAND_END); > +} This function has to be static, and like other static functions, it has to be added along with its first user, otherwise the project won't build with --enable-gcc-Werror. > +static char *drm_get_driver_name(struct tcb *tcp) > +{ > + char path[PATH_MAX]; > + char link[PATH_MAX]; > + int ret; > + > + if (getfdpath(tcp, tcp->u_arg[0], path, PATH_MAX - 1) < 0) > + return NULL; > + > + snprintf(link, PATH_MAX, "/sys/class/drm/%s/device/driver", > + basename(path)); if (snprintf(link, sizeof(link), ...) >= sizeof(link)) return NULL; > +static void drm_free_priv(void *data) > +{ > + free(data); > +} Do we really need a wrapper for free(3)? > --- a/ioctl.c > +++ b/ioctl.c > @@ -248,6 +248,10 @@ ioctl_decode(struct tcb *tcp) > case 0x22: > return scsi_ioctl(tcp, code, arg); > #endif > +#if defined(HAVE_DRM_H) || defined(HAVE_DRM_DRM_H) > + case 'd': > + return drm_ioctl(tcp, code, arg); > +#endif > case 'L': > return loop_ioctl(tcp, code, arg); > case 'M': v3 version also patched ioctl_decode_command_number() to call drm_decode_number().
On Tue, Sep 08, 2015 at 03:36:25AM +0300, Dmitry V. Levin wrote: > On Mon, Aug 24, 2015 at 02:42:48PM +0200, Patrik Jakobsson wrote: > > * Makefile.am: Add compilation of drm.c. > > * defs.h: Add extern declaration of drm_ioctl when drm headers are found. > > * drm.c: New file. > > * ioctl.c (ioctl_decode): Dispatch drm ioctls when drm headers are found. > > * defs.h (drm_decode_number, drm_ioctl): New prototypes. > * drm.c: New file. > * Makefile.am (strace_SOURCES): Add it. > * ioctl.c (ioctl_decode_command_number, ioctl_decode) > [HAVE_DRM_H || HAVE_DRM_DRM_H]: Dispatch drm ioctls. Ugh, thought I had this correct already. Will fix. > > --- a/defs.h > > +++ b/defs.h > > @@ -612,6 +612,9 @@ extern const char *sprint_open_modes(int); > > extern void print_seccomp_filter(struct tcb *tcp, unsigned long); > > > > extern int block_ioctl(struct tcb *, const unsigned int, long); > > +#if defined(HAVE_DRM_H) || defined(HAVE_DRM_DRM_H) > > +extern int drm_ioctl(struct tcb *, const unsigned int, long); > > +#endif > > I think function these prototypes could be added unconditionally. > Note that v3 version of this patch also declared drm_decode_number(). Ok. Will move the number decoding pieces back into this patch. Not sure why I did this in the first place. > > --- /dev/null > > +++ b/drm.c > [...] > > +#include "defs.h" > > + > > +#if defined(HAVE_DRM_H) || defined(HAVE_DRM_DRM_H) > > + > > +#ifdef HAVE_DRM_H > > +#include <drm.h> > > +#else > > +#include <drm/drm.h> > > +#endif > > + > > +#include <sys/param.h> > > Let's include <sys/param.h> before drm stuff. Ok > > +#define DRM_MAX_NAME_LEN 128 > > + > > +inline int drm_is_priv(const unsigned int num) > > +{ > > + return (_IOC_NR(num) >= DRM_COMMAND_BASE && > > + _IOC_NR(num) < DRM_COMMAND_END); > > +} > > This function has to be static, and like other static functions, > it has to be added along with its first user, otherwise the project > won't build with --enable-gcc-Werror. Will move it to the correct patch and make it static. > > +static char *drm_get_driver_name(struct tcb *tcp) > > +{ > > + char path[PATH_MAX]; > > + char link[PATH_MAX]; > > + int ret; > > + > > + if (getfdpath(tcp, tcp->u_arg[0], path, PATH_MAX - 1) < 0) > > + return NULL; > > + > > + snprintf(link, PATH_MAX, "/sys/class/drm/%s/device/driver", > > + basename(path)); > > if (snprintf(link, sizeof(link), ...) >= sizeof(link)) > return NULL; > According to manpage snprintf never returns more than the specified size - 1. So the only error we can check for is: if (snprintf(link, sizeof(link), ...) < sizeof("/sys/class/drm/%s/device/driver")) > > +static void drm_free_priv(void *data) > > +{ > > + free(data); > > +} > > Do we really need a wrapper for free(3)? No. The only reason I see is for clarity on how to use the tcb priv interface. But that is ofc debatable. > > --- a/ioctl.c > > +++ b/ioctl.c > > @@ -248,6 +248,10 @@ ioctl_decode(struct tcb *tcp) > > case 0x22: > > return scsi_ioctl(tcp, code, arg); > > #endif > > +#if defined(HAVE_DRM_H) || defined(HAVE_DRM_DRM_H) > > + case 'd': > > + return drm_ioctl(tcp, code, arg); > > +#endif > > case 'L': > > return loop_ioctl(tcp, code, arg); > > case 'M': > > v3 version also patched ioctl_decode_command_number() > to call drm_decode_number(). Will move it back into this patch. > > > -- > ldv
On Fri, Sep 11, 2015 at 12:57:06PM +0200, Patrik Jakobsson wrote: > On Tue, Sep 08, 2015 at 03:36:25AM +0300, Dmitry V. Levin wrote: > > On Mon, Aug 24, 2015 at 02:42:48PM +0200, Patrik Jakobsson wrote: [...] > > > +static char *drm_get_driver_name(struct tcb *tcp) > > > +{ > > > + char path[PATH_MAX]; > > > + char link[PATH_MAX]; > > > + int ret; > > > + > > > + if (getfdpath(tcp, tcp->u_arg[0], path, PATH_MAX - 1) < 0) > > > + return NULL; > > > + > > > + snprintf(link, PATH_MAX, "/sys/class/drm/%s/device/driver", > > > + basename(path)); > > > > if (snprintf(link, sizeof(link), ...) >= sizeof(link)) > > return NULL; > > > > According to manpage snprintf never returns more than the specified size - 1. Really? "The functions snprintf() and vsnprintf() do not write more than size bytes (including the terminating null byte ('\0')). If the output was truncated due to this limit, then the return value is the number of characters (excluding the terminating null byte) which would have been written to the final string if enough space had been available. Thus, a return value of size or more means that the output was truncated."
diff --git a/Makefile.am b/Makefile.am index f70f6d2..c7c6080 100644 --- a/Makefile.am +++ b/Makefile.am @@ -42,6 +42,7 @@ strace_SOURCES = \ count.c \ desc.c \ dirent.c \ + drm.c \ epoll.c \ evdev.c \ eventfd.c \ diff --git a/defs.h b/defs.h index bc3bd83..dd3c720 100644 --- a/defs.h +++ b/defs.h @@ -612,6 +612,9 @@ extern const char *sprint_open_modes(int); extern void print_seccomp_filter(struct tcb *tcp, unsigned long); extern int block_ioctl(struct tcb *, const unsigned int, long); +#if defined(HAVE_DRM_H) || defined(HAVE_DRM_DRM_H) +extern int drm_ioctl(struct tcb *, const unsigned int, long); +#endif extern int evdev_ioctl(struct tcb *, const unsigned int, long); extern int loop_ioctl(struct tcb *, const unsigned int, long); extern int mtd_ioctl(struct tcb *, const unsigned int, long); diff --git a/drm.c b/drm.c new file mode 100644 index 0000000..0829803 --- /dev/null +++ b/drm.c @@ -0,0 +1,107 @@ +/* + * Copyright (c) 2015 Intel Corporation + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. The name of the author may not be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + * Authors: + * Patrik Jakobsson <patrik.jakobsson@linux.intel.com> + */ + +#include "defs.h" + +#if defined(HAVE_DRM_H) || defined(HAVE_DRM_DRM_H) + +#ifdef HAVE_DRM_H +#include <drm.h> +#else +#include <drm/drm.h> +#endif + +#include <sys/param.h> + +#define DRM_MAX_NAME_LEN 128 + +inline int drm_is_priv(const unsigned int num) +{ + return (_IOC_NR(num) >= DRM_COMMAND_BASE && + _IOC_NR(num) < DRM_COMMAND_END); +} + +static char *drm_get_driver_name(struct tcb *tcp) +{ + char path[PATH_MAX]; + char link[PATH_MAX]; + int ret; + + if (getfdpath(tcp, tcp->u_arg[0], path, PATH_MAX - 1) < 0) + return NULL; + + snprintf(link, PATH_MAX, "/sys/class/drm/%s/device/driver", + basename(path)); + + ret = readlink(link, path, PATH_MAX - 1); + if (ret < 0) + return NULL; + + path[ret] = '\0'; + return strdup(basename(path)); +} + +static void drm_free_priv(void *data) +{ + free(data); +} + +int drm_is_driver(struct tcb *tcp, const char *name) +{ + char *priv; + + /* + * If no private data is allocated we are detecting the driver name for + * the first time and must resolve it. + */ + if (tcp->priv_data == NULL) { + tcp->priv_data = drm_get_driver_name(tcp); + if (tcp->priv_data == NULL) + return 0; + + tcp->free_priv_data = drm_free_priv; + } + + priv = tcp->priv_data; + + return strncmp(name, priv, DRM_MAX_NAME_LEN) == 0; +} + +int drm_decode_number(struct tcb *tcp, unsigned int code) +{ + return 0; +} + +int drm_ioctl(struct tcb *tcp, const unsigned int code, long arg) +{ + return 0; +} + +#endif /* HAVE_DRM_H || HAVE_DRM_DRM_H */ diff --git a/ioctl.c b/ioctl.c index 284828a..dfd35d8 100644 --- a/ioctl.c +++ b/ioctl.c @@ -248,6 +248,10 @@ ioctl_decode(struct tcb *tcp) case 0x22: return scsi_ioctl(tcp, code, arg); #endif +#if defined(HAVE_DRM_H) || defined(HAVE_DRM_DRM_H) + case 'd': + return drm_ioctl(tcp, code, arg); +#endif case 'L': return loop_ioctl(tcp, code, arg); case 'M':
* Makefile.am: Add compilation of drm.c. * defs.h: Add extern declaration of drm_ioctl when drm headers are found. * drm.c: New file. * ioctl.c (ioctl_decode): Dispatch drm ioctls when drm headers are found. Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> --- Makefile.am | 1 + defs.h | 3 ++ drm.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ ioctl.c | 4 +++ 4 files changed, 115 insertions(+) create mode 100644 drm.c