[PATCH] libbb: in xmalloc_fgetline, use getline if enabled

2023-05-28 Thread Elvira Khabirova
When xmalloc_fgetline was introduced, getline(3) was a GNU extension
and was not necessarily implemented in libcs. Since then,
it's become a part of POSIX.1-2008 and is now implemented in
glibc, uClibc-ng, and musl.

Previously, xmalloc_fgetline directly called bb_get_chunk_from_file.
The issue with that approach is that bb_get_chunk_from_file stops
both at \n and at \0. This introduces unexpected behavior when tools
are presented with inputs containing \0. For example, in a comparison
of GNU core utils cut with busybox cut:

% echo -ne '\x00\x01\x00\x03\x04\x05\x06' | cut -b1-3 | hexdump -C
  00 01 00 0a   ||
0004
% echo -ne '\x00\x01\x00\x03\x04\x05\x06' | ./busybox cut -b1-3 | hexdump -C
  0a 01 0a 03 04 05 0a  |...|
0007

Here, due to bb_get_chunk_from_file splitting lines by \n and \0,
busybox cut interprets the input as three lines instead of one.

Also, since xmalloc_fgetline never returned strings with embedded \0,
cut_file expects strlen of the returned string to match the string's
total length.

To fix the behavior of the cut utility, introduce xmalloc_fgetline_n,
that fully matches the behavior of xmalloc_fgetline,
but also returns the amount of bytes read.

Add a configuration option, FEATURE_USE_GETLINE, and enable it
by default. Use getline in xmalloc_fgetline_n if the feature is enabled.

Change the behavior of cut_file to use the values returned
by the new function instead of calling strlen.

Call xmalloc_fgetline_n from xmalloc_fgetline.

Add a test for the previously mentioned case.

With FEATURE_USE_GETLINE:

function old new   delta
xmalloc_fgetline_n - 173+173
xmalloc_fgetline  85  58 -27
cut_main14061367 -39
--
(add/remove: 1/0 grow/shrink: 0/2 up/down: 173/-66)   Total: 107 bytes

Without FEATURE_USE_GETLINE:

function old new   delta
xmalloc_fgetline_n -  41 +41
xmalloc_fgetline  85  58 -27
cut_main14061367 -39
--
(add/remove: 1/0 grow/shrink: 0/2 up/down: 41/-66)Total: -25 bytes

Fixes: https://bugs.busybox.net/show_bug.cgi?id=15276
Signed-off-by: Elvira Khabirova 
---
 coreutils/cut.c|  4 ++--
 include/libbb.h|  2 ++
 libbb/Config.src   | 11 +++
 libbb/get_line_from_file.c | 37 -
 testsuite/cut.tests|  2 ++
 5 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/coreutils/cut.c b/coreutils/cut.c
index 55bdd9386..7c87467ca 100644
--- a/coreutils/cut.c
+++ b/coreutils/cut.c
@@ -89,6 +89,7 @@ static void cut_file(FILE *file, const char *delim, const 
char *odelim,
const struct cut_list *cut_lists, unsigned nlists)
 {
char *line;
+   size_t linelen = 0;
unsigned linenum = 0;   /* keep these zero-based to be consistent */
regex_t reg;
int spos, shoe = option_mask32 & CUT_OPT_REGEX_FLGS;
@@ -96,10 +97,9 @@ static void cut_file(FILE *file, const char *delim, const 
char *odelim,
if (shoe) xregcomp(, delim, REG_EXTENDED);
 
/* go through every line in the file */
-   while ((line = xmalloc_fgetline(file)) != NULL) {
+   while ((line = xmalloc_fgetline_n(file, )) != NULL) {
 
/* set up a list so we can keep track of what's been printed */
-   int linelen = strlen(line);
char *printed = xzalloc(linelen + 1);
char *orig_line = line;
unsigned cl_pos = 0;
diff --git a/include/libbb.h b/include/libbb.h
index 6191debb1..73d16647a 100644
--- a/include/libbb.h
+++ b/include/libbb.h
@@ -1048,6 +1048,8 @@ extern char *xmalloc_fgetline_str(FILE *file, const char 
*terminating_string) FA
 extern char *xmalloc_fgets(FILE *file) FAST_FUNC RETURNS_MALLOC;
 /* Chops off '\n' from the end, unlike fgets: */
 extern char *xmalloc_fgetline(FILE *file) FAST_FUNC RETURNS_MALLOC;
+/* Same as xmalloc_fgetline but returns number of bytes read: */
+extern char* xmalloc_fgetline_n(FILE *file, size_t *n) FAST_FUNC 
RETURNS_MALLOC;
 /* Same, but doesn't try to conserve space (may have some slack after the end) 
*/
 /* extern char *xmalloc_fgetline_fast(FILE *file) FAST_FUNC RETURNS_MALLOC; */
 
diff --git a/libbb/Config.src b/libbb/Config.src
index b980f19a9..7a37929d6 100644
--- a/libbb/Config.src
+++ b/libbb/Config.src
@@ -147,6 +147,17 @@ config MONOTONIC_SYSCALL
will be used instead (which gives wrong results if date/

[PATCH v4 RESEND] tee: add support for application-based session login methods

2020-12-10 Thread Elvira Khabirova
This is both a resend and a request for comments.

The main reason for implementing this was to be able to authorize
access to particular TAs based on the applications that request it.
Furthermore, being able to distinguish between different applications
also allows having trusted storage per-application.
I believe this functionality might be crucial to many users of op-tee.

This patch provides two possible ways of calculating the application
identifier strings. This also serves as a stub for other methods.
Since there is no concept of "application" known to the Linux kernel,
the two proposed methods are based on the calling task's executable:
the executable file's path and its SELinux attributes.
Some vendor-specific methods may employ a service running in userspace,
but these two methods are the only ones that we came up with that
are fully contained in the kernel and are usable (and we actually use
one of the two methods suggested).

There might be other valid definitions of application-based identifier
strings. The GP TEE Client API specifies the credentials generated as
"only depending on the identity of the application program", being
"persistent within one implementation, across multiple invocations of
the application and across power cycles, enabling them to be used to
disambiguate persistent storage"; not more specific than that.

Perhaps some other properties of a running task can be used to calculate
an app ID, for example, to not depend on having an executable file
(which is admittedly a rare thing to come by), or to be able
to distinguish between different scripts run with one interpreter
(which is also rare since the TEE Client API is C-based).

If you're interested in this patch, or if you're using application-based
login methods but with a different scheme, I would love to hear your
experiences. Maybe there are other methods to consider; maybe the
proposed methods are enough.

If you're interested in testing this patch out, there is a reviewed,
good-to-go pull request in the optee_test repository:
https://github.com/OP-TEE/optee_test/pull/468

>8--8<

GP TEE Client API in addition to login methods already supported
in the kernel also defines several application-based methods:
TEEC_LOGIN_APPLICATION, TEEC_LOGIN_USER_APPLICATION, and
TEEC_LOGIN_GROUP_APPLICATION.

It specifies credentials generated for TEEC_LOGIN_APPLICATION as only
depending on the identity of the program, being persistent within one
implementation, across multiple invocations of the application
and across power cycles, enabling them to be used to disambiguate
persistent storage. The exact nature is REE-specific.

As the exact method of generating application identifier strings may
vary between vendors, setups and installations, add two suggested
methods and an exact framework for vendors to extend upon.

Signed-off-by: Elvira Khabirova 
---
Changes in v4:
- Fix potential exe_file leaks.

Changes in v3:
- Remove free_app_id() and replace it with calls to kfree().

Changes in v2:
- Rename some functions and variables to make them shorter.
- Include linux/security.h unconditionally.
- Restructure error handling in tee_session_calc_client_uuid().
---
 drivers/tee/Kconfig|  29 ++
 drivers/tee/tee_core.c | 126 ++---
 2 files changed, 147 insertions(+), 8 deletions(-)

diff --git a/drivers/tee/Kconfig b/drivers/tee/Kconfig
index e99d840c2511..4cd6e0d2aad5 100644
--- a/drivers/tee/Kconfig
+++ b/drivers/tee/Kconfig
@@ -11,6 +11,35 @@ config TEE
  This implements a generic interface towards a Trusted Execution
  Environment (TEE).
 
+choice
+   prompt "Application ID for client UUID"
+   depends on TEE
+   default TEE_APPID_PATH
+   help
+ This option allows to choose which method will be used to generate
+ application identifiers for client UUID generation when login methods
+ TEE_LOGIN_APPLICATION, TEE_LOGIN_USER_APPLICATION
+ and TEE_LOGIN_GROUP_APPLICATION are used.
+ Please be mindful of the security of each method in your particular
+ installation.
+
+   config TEE_APPID_PATH
+   bool "Path-based application ID"
+   help
+ Use the executable's path as an application ID.
+
+   config TEE_APPID_SECURITY
+   bool "Security extended attribute based application ID"
+   help
+ Use the executable's security extended attribute as an 
application ID.
+endchoice
+
+config TEE_APPID_SECURITY_XATTR
+   string "Security extended attribute to use for application ID"
+   depends on TEE_APPID_SECURITY
+   help
+ Attribute to be used as an application ID (with the security prefix 
removed).
+
 if TEE
 
 menu "TEE drivers"
diff --git a/drivers/tee/tee_core.c b/drivers/tee

[PATCH v4] tee: add support for application-based session login methods

2020-10-17 Thread Elvira Khabirova
GP TEE Client API in addition to login methods already supported
in the kernel also defines several application-based methods:
TEEC_LOGIN_APPLICATION, TEEC_LOGIN_USER_APPLICATION, and
TEEC_LOGIN_GROUP_APPLICATION.

It specifies credentials generated for TEEC_LOGIN_APPLICATION as only
depending on the identity of the program, being persistent within one
implementation, across multiple invocations of the application
and across power cycles, enabling them to be used to disambiguate
persistent storage. The exact nature is REE-specific.

As the exact method of generating application identifier strings may
vary between vendors, setups and installations, add two suggested
methods and an exact framework for vendors to extend upon.

Signed-off-by: Elvira Khabirova 
---
Changes in v4:
- Fix potential exe_file leaks.

Changes in v3:
- Remove free_app_id() and replace it with calls to kfree().

Changes in v2:
- Rename some functions and variables to make them shorter.
- Include linux/security.h unconditionally.
- Restructure error handling in tee_session_calc_client_uuid().
---
 drivers/tee/Kconfig|  29 ++
 drivers/tee/tee_core.c | 126 ++---
 2 files changed, 147 insertions(+), 8 deletions(-)

diff --git a/drivers/tee/Kconfig b/drivers/tee/Kconfig
index e99d840c2511..4cd6e0d2aad5 100644
--- a/drivers/tee/Kconfig
+++ b/drivers/tee/Kconfig
@@ -11,6 +11,35 @@ config TEE
  This implements a generic interface towards a Trusted Execution
  Environment (TEE).
 
+choice
+   prompt "Application ID for client UUID"
+   depends on TEE
+   default TEE_APPID_PATH
+   help
+ This option allows to choose which method will be used to generate
+ application identifiers for client UUID generation when login methods
+ TEE_LOGIN_APPLICATION, TEE_LOGIN_USER_APPLICATION
+ and TEE_LOGIN_GROUP_APPLICATION are used.
+ Please be mindful of the security of each method in your particular
+ installation.
+
+   config TEE_APPID_PATH
+   bool "Path-based application ID"
+   help
+ Use the executable's path as an application ID.
+
+   config TEE_APPID_SECURITY
+   bool "Security extended attribute based application ID"
+   help
+ Use the executable's security extended attribute as an 
application ID.
+endchoice
+
+config TEE_APPID_SECURITY_XATTR
+   string "Security extended attribute to use for application ID"
+   depends on TEE_APPID_SECURITY
+   help
+ Attribute to be used as an application ID (with the security prefix 
removed).
+
 if TEE
 
 menu "TEE drivers"
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
index 64637e09a095..a72b8c19253a 100644
--- a/drivers/tee/tee_core.c
+++ b/drivers/tee/tee_core.c
@@ -7,9 +7,12 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
+#include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -21,7 +24,7 @@
 
 #define TEE_IOCTL_PARAM_SIZE(x) (sizeof(struct tee_param) * (x))
 
-#define TEE_UUID_NS_NAME_SIZE  128
+#define TEE_UUID_NS_NAME_SIZE  PATH_MAX
 
 /*
  * TEE Client UUID name space identifier (UUIDv4)
@@ -125,6 +128,65 @@ static int tee_release(struct inode *inode, struct file 
*filp)
return 0;
 }
 
+#ifdef CONFIG_TEE_APPID_SECURITY
+static const char *get_app_id(void **data)
+{
+   struct file *exe_file;
+   const char *name = CONFIG_TEE_APPID_SECURITY_XATTR;
+   int len;
+
+   exe_file = get_mm_exe_file(current->mm);
+   if (!exe_file)
+   return ERR_PTR(-ENOENT);
+
+   if (!exe_file->f_inode) {
+   fput(exe_file);
+   return ERR_PTR(-ENOENT);
+   }
+
+   /*
+* An identifier string for the binary. Depends on the implementation.
+* Could be, for example, a string containing the application vendor ID,
+* or the binary's signature, or its hash and a timestamp.
+*/
+   len = security_inode_getsecurity(exe_file->f_inode, name, data, true);
+   fput(exe_file);
+
+   if (len < 0)
+   return ERR_PTR(len);
+
+   return *data;
+}
+#endif /* CONFIG_TEE_APPID_SECURITY */
+
+#ifdef CONFIG_TEE_APPID_PATH
+static const char *get_app_id(void **data)
+{
+   struct file *exe_file;
+   char *path;
+
+   *data = kzalloc(TEE_UUID_NS_NAME_SIZE, GFP_KERNEL);
+   if (!*data)
+   return ERR_PTR(-ENOMEM);
+
+   exe_file = get_mm_exe_file(current->mm);
+   if (!exe_file) {
+   kfree(*data);
+   return ERR_PTR(-ENOENT);
+   }
+
+   path = file_path(exe_file, *data, TEE_UUID_NS_NAME_SIZE);
+   fput(exe_file);
+
+   if (IS_ERR(path)) {
+   kfree(*data);
+   return path;
+   }
+
+   return path;
+}
+#endif /* CONFIG_TEE_APPID_PATH */
+
 /**
  * uuid_v5() - C

[PATCH v3] tee: add support for application-based session login methods

2020-10-05 Thread Elvira Khabirova
GP TEE Client API in addition to login methods already supported
in the kernel also defines several application-based methods:
TEEC_LOGIN_APPLICATION, TEEC_LOGIN_USER_APPLICATION, and
TEEC_LOGIN_GROUP_APPLICATION.

It specifies credentials generated for TEEC_LOGIN_APPLICATION as only
depending on the identity of the program, being persistent within one
implementation, across multiple invocations of the application
and across power cycles, enabling them to be used to disambiguate
persistent storage. The exact nature is REE-specific.

As the exact method of generating application identifier strings may
vary between vendors, setups and installations, add two suggested
methods and an exact framework for vendors to extend upon.

Signed-off-by: Elvira Khabirova 
---
Jens, do you have any advice on finding more reviewers for this?

Changes in v3:
- Remove free_app_id() and replace it with calls to kfree().

Changes in v2:
- Rename some functions and variables to make them shorter.
- Include linux/security.h unconditionally.
- Restructure error handling in tee_session_calc_client_uuid().

 drivers/tee/Kconfig|  29 ++
 drivers/tee/tee_core.c | 126 ++---
 2 files changed, 147 insertions(+), 8 deletions(-)

diff --git a/drivers/tee/Kconfig b/drivers/tee/Kconfig
index e99d840c2511..4cd6e0d2aad5 100644
--- a/drivers/tee/Kconfig
+++ b/drivers/tee/Kconfig
@@ -11,6 +11,35 @@ config TEE
  This implements a generic interface towards a Trusted Execution
  Environment (TEE).
 
+choice
+   prompt "Application ID for client UUID"
+   depends on TEE
+   default TEE_APPID_PATH
+   help
+ This option allows to choose which method will be used to generate
+ application identifiers for client UUID generation when login methods
+ TEE_LOGIN_APPLICATION, TEE_LOGIN_USER_APPLICATION
+ and TEE_LOGIN_GROUP_APPLICATION are used.
+ Please be mindful of the security of each method in your particular
+ installation.
+
+   config TEE_APPID_PATH
+   bool "Path-based application ID"
+   help
+ Use the executable's path as an application ID.
+
+   config TEE_APPID_SECURITY
+   bool "Security extended attribute based application ID"
+   help
+ Use the executable's security extended attribute as an 
application ID.
+endchoice
+
+config TEE_APPID_SECURITY_XATTR
+   string "Security extended attribute to use for application ID"
+   depends on TEE_APPID_SECURITY
+   help
+ Attribute to be used as an application ID (with the security prefix 
removed).
+
 if TEE
 
 menu "TEE drivers"
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
index 64637e09a095..3626ce0d9bd7 100644
--- a/drivers/tee/tee_core.c
+++ b/drivers/tee/tee_core.c
@@ -7,9 +7,12 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
+#include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -21,7 +24,7 @@
 
 #define TEE_IOCTL_PARAM_SIZE(x) (sizeof(struct tee_param) * (x))
 
-#define TEE_UUID_NS_NAME_SIZE  128
+#define TEE_UUID_NS_NAME_SIZE  PATH_MAX
 
 /*
  * TEE Client UUID name space identifier (UUIDv4)
@@ -125,6 +128,65 @@ static int tee_release(struct inode *inode, struct file 
*filp)
return 0;
 }
 
+#ifdef CONFIG_TEE_APPID_SECURITY
+static const char *get_app_id(void **data)
+{
+   struct file *exe_file;
+   const char *name = CONFIG_TEE_APPID_SECURITY_XATTR;
+   int len;
+
+   exe_file = get_mm_exe_file(current->mm);
+   if (!exe_file)
+   return ERR_PTR(-ENOENT);
+
+   if (!exe_file->f_inode) {
+   fput(exe_file);
+   return ERR_PTR(-ENOENT);
+   }
+
+   /*
+* An identifier string for the binary. Depends on the implementation.
+* Could be, for example, a string containing the application vendor ID,
+* or the binary's signature, or its hash and a timestamp.
+*/
+   len = security_inode_getsecurity(exe_file->f_inode, name, data, true);
+   if (len < 0)
+   return ERR_PTR(len);
+
+   fput(exe_file);
+
+   return *data;
+}
+#endif /* CONFIG_TEE_APPID_SECURITY */
+
+#ifdef CONFIG_TEE_APPID_PATH
+static const char *get_app_id(void **data)
+{
+   struct file *exe_file;
+   char *path;
+
+   *data = kzalloc(TEE_UUID_NS_NAME_SIZE, GFP_KERNEL);
+   if (!*data)
+   return ERR_PTR(-ENOMEM);
+
+   exe_file = get_mm_exe_file(current->mm);
+   if (!exe_file) {
+   kfree(*data);
+   return ERR_PTR(-ENOENT);
+   }
+
+   path = file_path(exe_file, *data, TEE_UUID_NS_NAME_SIZE);
+   if (IS_ERR(path)) {
+   kfree(*data);
+   return path;
+   }
+
+   fput(exe_file);
+
+   return path;
+}
+#endif /* CONFIG_TEE_APPID_PATH */
+
 /**
  * uuid_v

Re: [PATCH v2] tee: add support for application-based session login methods

2020-10-01 Thread Elvira Khabirova
On Thu, 1 Oct 2020 11:47:58 +0200
Jens Wiklander  wrote:

> On Wed, Sep 30, 2020 at 07:56:52PM +0300, Elvira Khabirova wrote:
> > +#if defined(CONFIG_TEE_APPID_PATH) || defined(CONFIG_TEE_APPID_SECURITY)
> > +static void free_app_id(void *data)
> > +{
> > +   kfree(data);
> > +}
> > +#endif /* CONFIG_TEE_APPID_PATH || CONFIG_TEE_APPID_SECURITY */
> I don't think we need this function, we could just as well use kfree()
> directly instead.

Ah, I supposed "use the pattern you're already using" meant to keep
the freeing function too. My bad.

> Thanks,
> Jens
> 
> > +
> >  /**
> >   * uuid_v5() - Calculate UUIDv5
> >   * @uuid: Resulting UUID
> > @@ -208,6 +277,8 @@ int tee_session_calc_client_uuid(uuid_t *uuid, u32 
> > connection_method,
> > gid_t ns_grp = (gid_t)-1;
> > kgid_t grp = INVALID_GID;
> > char *name = NULL;
> > +   void *app_id_data = NULL;
> > +   const char *app_id = NULL;
> > int name_len;
> > int rc;
> >  
> > @@ -228,6 +299,14 @@ int tee_session_calc_client_uuid(uuid_t *uuid, u32 
> > connection_method,
> >  * For TEEC_LOGIN_GROUP:
> >  * gid=
> >  *
> > +* For TEEC_LOGIN_APPLICATION:
> > +* app=
> > +*
> > +* For TEEC_LOGIN_USER_APPLICATION:
> > +* uid=:app=
> > +*
> > +* For TEEC_LOGIN_GROUP_APPLICATION:
> > +* gid=:app=
> >  */
> >  
> > name = kzalloc(TEE_UUID_NS_NAME_SIZE, GFP_KERNEL);
> > @@ -238,10 +317,6 @@ int tee_session_calc_client_uuid(uuid_t *uuid, u32 
> > connection_method,
> > case TEE_IOCTL_LOGIN_USER:
> > name_len = snprintf(name, TEE_UUID_NS_NAME_SIZE, "uid=%x",
> > current_euid().val);
> > -   if (name_len >= TEE_UUID_NS_NAME_SIZE) {
> > -   rc = -E2BIG;
> > -   goto out_free_name;
> > -   }
> > break;
> >  
> > case TEE_IOCTL_LOGIN_GROUP:
> > @@ -254,10 +329,49 @@ int tee_session_calc_client_uuid(uuid_t *uuid, u32 
> > connection_method,
> >  
> > name_len = snprintf(name, TEE_UUID_NS_NAME_SIZE, "gid=%x",
> > grp.val);
> > -   if (name_len >= TEE_UUID_NS_NAME_SIZE) {
> > -   rc = -E2BIG;
> > +   break;
> > +
> > +   case TEE_IOCTL_LOGIN_APPLICATION:
> > +   app_id = get_app_id(_id_data);
> > +   if (IS_ERR(app_id)) {
> > +   rc = PTR_ERR(app_id);
> > +   goto out_free_name;
> > +   }
> > +
> > +   name_len = snprintf(name, TEE_UUID_NS_NAME_SIZE, "app=%s",
> > +   app_id);
> > +   free_app_id(app_id_data);
> > +   break;
> > +
> > +   case TEE_IOCTL_LOGIN_USER_APPLICATION:
> > +   app_id = get_app_id(_id_data);
> > +   if (IS_ERR(app_id)) {
> > +   rc = PTR_ERR(app_id);
> > +   goto out_free_name;
> > +   }
> > +
> > +   name_len = snprintf(name, TEE_UUID_NS_NAME_SIZE, 
> > "uid=%x:app=%s",
> > +   current_euid().val, app_id);
> > +   free_app_id(app_id_data);
> > +   break;
> > +
> > +   case TEE_IOCTL_LOGIN_GROUP_APPLICATION:
> > +   memcpy(_grp, connection_data, sizeof(gid_t));
> > +   grp = make_kgid(current_user_ns(), ns_grp);
> > +   if (!gid_valid(grp) || !in_egroup_p(grp)) {
> > +   rc = -EPERM;
> > +   goto out_free_name;
> > +   }
> > +
> > +   app_id = get_app_id(_id_data);
> > +   if (IS_ERR(app_id)) {
> > +   rc = PTR_ERR(app_id);
> > goto out_free_name;
> > }
> > +
> > +   name_len = snprintf(name, TEE_UUID_NS_NAME_SIZE, 
> > "gid=%x:app=%s",
> > +   grp.val, app_id);
> > +   free_app_id(app_id_data);
> > break;
> >  
> > default:
> > @@ -265,7 +379,10 @@ int tee_session_calc_client_uuid(uuid_t *uuid, u32 
> > connection_method,
> > goto out_free_name;
> > }
> >  
> > -   rc = uuid_v5(uuid, _client_uuid_ns, name, name_len);
> > +   if (name_len < TEE_UUID_NS_NAME_SIZE)
> > +   rc = uuid_v5(uuid, _client_uuid_ns, name, name_len);
> > +   else
> > +   rc = -E2BIG;
> >  out_free_name:
> > kfree(name);
> >  
> > -- 
> > 2.17.1
> > 



[PATCH v2] tee: add support for application-based session login methods

2020-09-30 Thread Elvira Khabirova
GP TEE Client API in addition to login methods already supported
in the kernel also defines several application-based methods:
TEEC_LOGIN_APPLICATION, TEEC_LOGIN_USER_APPLICATION, and
TEEC_LOGIN_GROUP_APPLICATION.

It specifies credentials generated for TEEC_LOGIN_APPLICATION as only
depending on the identity of the program, being persistent within one
implementation, across multiple invocations of the application
and across power cycles, enabling them to be used to disambiguate
persistent storage. The exact nature is REE-specific.

As the exact method of generating application identifier strings may
vary between vendors, setups and installations, add two suggested
methods and an exact framework for vendors to extend upon.

Signed-off-by: Elvira Khabirova 
---
 drivers/tee/Kconfig|  29 +
 drivers/tee/tee_core.c | 133 ++---
 2 files changed, 154 insertions(+), 8 deletions(-)

diff --git a/drivers/tee/Kconfig b/drivers/tee/Kconfig
index 6488b66d69dd..cd8f02a3d488 100644
--- a/drivers/tee/Kconfig
+++ b/drivers/tee/Kconfig
@@ -10,6 +10,35 @@ config TEE
  This implements a generic interface towards a Trusted Execution
  Environment (TEE).
 
+choice
+   prompt "Application ID for client UUID"
+   depends on TEE
+   default TEE_APPID_PATH
+   help
+ This option allows to choose which method will be used to generate
+ application identifiers for client UUID generation when login methods
+ TEE_LOGIN_APPLICATION, TEE_LOGIN_USER_APPLICATION
+ and TEE_LOGIN_GROUP_APPLICATION are used.
+ Please be mindful of the security of each method in your particular
+ installation.
+
+   config TEE_APPID_PATH
+   bool "Path-based application ID"
+   help
+ Use the executable's path as an application ID.
+
+   config TEE_APPID_SECURITY
+   bool "Security extended attribute based application ID"
+   help
+ Use the executable's security extended attribute as an 
application ID.
+endchoice
+
+config TEE_APPID_SECURITY_XATTR
+   string "Security extended attribute to use for application ID"
+   depends on TEE_APPID_SECURITY
+   help
+ Attribute to be used as an application ID (with the security prefix 
removed).
+
 if TEE
 
 menu "TEE drivers"
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
index 67186d00a52f..31fb244b59ed 100644
--- a/drivers/tee/tee_core.c
+++ b/drivers/tee/tee_core.c
@@ -16,9 +16,12 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
+#include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -31,7 +34,7 @@
 
 #define TEE_IOCTL_PARAM_SIZE(x) (sizeof(struct tee_param) * (x))
 
-#define TEE_UUID_NS_NAME_SIZE  128
+#define TEE_UUID_NS_NAME_SIZE  PATH_MAX
 
 /*
  * TEE Client UUID name space identifier (UUIDv4)
@@ -136,6 +139,72 @@ static int tee_release(struct inode *inode, struct file 
*filp)
return 0;
 }
 
+#ifdef CONFIG_TEE_APPID_SECURITY
+static const char *get_app_id(void **data)
+{
+   struct file *exe_file;
+   const char *name = CONFIG_TEE_APPID_SECURITY_XATTR;
+   int len;
+
+   exe_file = get_mm_exe_file(current->mm);
+   if (!exe_file)
+   return ERR_PTR(-ENOENT);
+
+   if (!exe_file->f_inode) {
+   fput(exe_file);
+   return ERR_PTR(-ENOENT);
+   }
+
+   /*
+* An identifier string for the binary. Depends on the implementation.
+* Could be, for example, a string containing the application vendor ID,
+* or the binary's signature, or its hash and a timestamp.
+*/
+   len = security_inode_getsecurity(exe_file->f_inode, name, data, true);
+   if (len < 0)
+   return ERR_PTR(len);
+
+   fput(exe_file);
+
+   return *data;
+}
+#endif /* CONFIG_TEE_APPID_SECURITY */
+
+#ifdef CONFIG_TEE_APPID_PATH
+static const char *get_app_id(void **data)
+{
+   struct file *exe_file;
+   char *path;
+
+   *data = kzalloc(TEE_UUID_NS_NAME_SIZE, GFP_KERNEL);
+   if (!*data)
+   return ERR_PTR(-ENOMEM);
+
+   exe_file = get_mm_exe_file(current->mm);
+   if (!exe_file) {
+   kfree(*data);
+   return ERR_PTR(-ENOENT);
+   }
+
+   path = file_path(exe_file, *data, TEE_UUID_NS_NAME_SIZE);
+   if (IS_ERR(path)) {
+   kfree(*data);
+   return path;
+   }
+
+   fput(exe_file);
+
+   return path;
+}
+#endif /* CONFIG_TEE_APPID_PATH */
+
+#if defined(CONFIG_TEE_APPID_PATH) || defined(CONFIG_TEE_APPID_SECURITY)
+static void free_app_id(void *data)
+{
+   kfree(data);
+}
+#endif /* CONFIG_TEE_APPID_PATH || CONFIG_TEE_APPID_SECURITY */
+
 /**
  * uuid_v5() - Calculate UUIDv5
  * @uuid: Resulting UUID
@@ -208,6 +277,8 @@ int tee_session_calc_client_uuid(uuid_t *uuid, u32 
co

Re: [RFC PATCH] tee: add support for application-based session login methods

2020-09-29 Thread Elvira Khabirova
On Mon, 28 Sep 2020 15:43:47 +0200
Jens Wiklander  wrote:

> Hi Elvira,
> 
> On Thu, Sep 17, 2020 at 06:38:03PM +0300, Elvira Khabirova wrote:
> > GP TEE Client API in addition to login methods already supported
> > in the kernel also defines several application-based methods:
> > TEEC_LOGIN_APPLICATION, TEEC_LOGIN_USER_APPLICATION, and
> > TEEC_LOGIN_GROUP_APPLICATION.
> > 
> > It specifies credentials generated for TEEC_LOGIN_APPLICATION as only
> > depending on the identity of the program, being persistent within one
> > implementation, across multiple invocations of the application
> > and across power cycles, enabling them to be used to disambiguate
> > persistent storage. The exact nature is REE-specific.
> > 
> > As the exact method of generating application identifier strings may
> > vary between vendors, setups and installations, add two suggested
> > methods and an exact framework for vendors to extend upon.
> > 
> > Signed-off-by: Elvira Khabirova 
> > ---
> >  drivers/tee/Kconfig|  29 +
> >  drivers/tee/tee_core.c | 136 -
> >  2 files changed, 164 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/tee/Kconfig b/drivers/tee/Kconfig
> > index e99d840c2511..4cd6e0d2aad5 100644
> > --- a/drivers/tee/Kconfig
> > +++ b/drivers/tee/Kconfig
> > @@ -11,6 +11,35 @@ config TEE
> >   This implements a generic interface towards a Trusted Execution
> >   Environment (TEE).
> >  
> > +choice
> > +   prompt "Application ID for client UUID"
> > +   depends on TEE
> > +   default TEE_APPID_PATH
> > +   help
> > + This option allows to choose which method will be used to generate
> > + application identifiers for client UUID generation when login methods
> > + TEE_LOGIN_APPLICATION, TEE_LOGIN_USER_APPLICATION
> > + and TEE_LOGIN_GROUP_APPLICATION are used.
> > + Please be mindful of the security of each method in your particular
> > + installation.
> > +
> > +   config TEE_APPID_PATH
> > +   bool "Path-based application ID"
> > +   help
> > + Use the executable's path as an application ID.
> > +
> > +   config TEE_APPID_SECURITY
> > +   bool "Security extended attribute based application ID"
> > +   help
> > + Use the executable's security extended attribute as an 
> > application ID.
> > +endchoice
> > +
> > +config TEE_APPID_SECURITY_XATTR
> > +   string "Security extended attribute to use for application ID"
> > +   depends on TEE_APPID_SECURITY
> > +   help
> > + Attribute to be used as an application ID (with the security prefix 
> > removed).
> > +
> >  if TEE
> >  
> >  menu "TEE drivers"
> > diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> > index 64637e09a095..19c965dd212b 100644
> > --- a/drivers/tee/tee_core.c
> > +++ b/drivers/tee/tee_core.c
> > @@ -7,8 +7,10 @@
> >  
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -17,11 +19,15 @@
> >  #include 
> >  #include "tee_private.h"
> >  
> > +#ifdef CONFIG_TEE_APPID_SECURITY
> No need for the ifdef, just inlude the file unconditionally above.
> 
> > +#include 
> > +#endif
> > +
> >  #define TEE_NUM_DEVICES32
> >  
> >  #define TEE_IOCTL_PARAM_SIZE(x) (sizeof(struct tee_param) * (x))
> >  
> > -#define TEE_UUID_NS_NAME_SIZE  128
> > +#define TEE_UUID_NS_NAME_SIZE  PATH_MAX
> >  
> >  /*
> >   * TEE Client UUID name space identifier (UUIDv4)
> > @@ -125,6 +131,67 @@ static int tee_release(struct inode *inode, struct 
> > file *filp)
> > return 0;
> >  }
> >  
> > +#ifdef CONFIG_TEE_APPID_SECURITY
> > +static const char *tee_session_get_application_id(void **data)
> static char *get_app_id(void) should be enough.
> 
> > +{
> > +   struct file *exe_file;
> > +   const char *name = CONFIG_TEE_APPID_SECURITY_XATTR;
> > +   int len;
> > +
> > +   exe_file = get_mm_exe_file(current->mm);
> > +   if (!exe_file)
> > +   return ERR_PTR(-ENOENT);
> > +
> > +   if (!exe_file->f_inode) {
> > +   fput(exe_file);
> > +   return ERR_PTR(-ENOENT);
> > +   }
> > +
> 
> You could perhaps add a comm

Re: [RFC PATCH] tee: add support for application-based session login methods

2020-09-28 Thread Elvira Khabirova
Hello,

On Thu, 17 Sep 2020 15:46:07 +
Elvira Khabirova via OP-TEE  wrote:

> GP TEE Client API in addition to login methods already supported
> in the kernel also defines several application-based methods:
> TEEC_LOGIN_APPLICATION, TEEC_LOGIN_USER_APPLICATION, and
> TEEC_LOGIN_GROUP_APPLICATION.
> 
> It specifies credentials generated for TEEC_LOGIN_APPLICATION as only
> depending on the identity of the program, being persistent within one
> implementation, across multiple invocations of the application
> and across power cycles, enabling them to be used to disambiguate
> persistent storage. The exact nature is REE-specific.
> 
> As the exact method of generating application identifier strings may
> vary between vendors, setups and installations, add two suggested
> methods and an exact framework for vendors to extend upon.

If there are no comments on this, I will resend this and drop [RFC]
from the subject.

> Signed-off-by: Elvira Khabirova 
> ---
>  drivers/tee/Kconfig|  29 +
>  drivers/tee/tee_core.c | 136 -
>  2 files changed, 164 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tee/Kconfig b/drivers/tee/Kconfig
> index e99d840c2511..4cd6e0d2aad5 100644
> --- a/drivers/tee/Kconfig
> +++ b/drivers/tee/Kconfig
> @@ -11,6 +11,35 @@ config TEE
> This implements a generic interface towards a Trusted Execution
> Environment (TEE).
>  
> +choice
> + prompt "Application ID for client UUID"
> + depends on TEE
> + default TEE_APPID_PATH
> + help
> +   This option allows to choose which method will be used to generate
> +   application identifiers for client UUID generation when login methods
> +   TEE_LOGIN_APPLICATION, TEE_LOGIN_USER_APPLICATION
> +   and TEE_LOGIN_GROUP_APPLICATION are used.
> +   Please be mindful of the security of each method in your particular
> +   installation.
> +
> + config TEE_APPID_PATH
> + bool "Path-based application ID"
> + help
> +   Use the executable's path as an application ID.
> +
> + config TEE_APPID_SECURITY
> + bool "Security extended attribute based application ID"
> + help
> +   Use the executable's security extended attribute as an 
> application ID.
> +endchoice
> +
> +config TEE_APPID_SECURITY_XATTR
> + string "Security extended attribute to use for application ID"
> + depends on TEE_APPID_SECURITY
> + help
> +   Attribute to be used as an application ID (with the security prefix 
> removed).
> +
>  if TEE
>  
>  menu "TEE drivers"
> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> index 64637e09a095..19c965dd212b 100644
> --- a/drivers/tee/tee_core.c
> +++ b/drivers/tee/tee_core.c
> @@ -7,8 +7,10 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -17,11 +19,15 @@
>  #include 
>  #include "tee_private.h"
>  
> +#ifdef CONFIG_TEE_APPID_SECURITY
> +#include 
> +#endif
> +
>  #define TEE_NUM_DEVICES  32
>  
>  #define TEE_IOCTL_PARAM_SIZE(x) (sizeof(struct tee_param) * (x))
>  
> -#define TEE_UUID_NS_NAME_SIZE128
> +#define TEE_UUID_NS_NAME_SIZEPATH_MAX
>  
>  /*
>   * TEE Client UUID name space identifier (UUIDv4)
> @@ -125,6 +131,67 @@ static int tee_release(struct inode *inode, struct file 
> *filp)
>   return 0;
>  }
>  
> +#ifdef CONFIG_TEE_APPID_SECURITY
> +static const char *tee_session_get_application_id(void **data)
> +{
> + struct file *exe_file;
> + const char *name = CONFIG_TEE_APPID_SECURITY_XATTR;
> + int len;
> +
> + exe_file = get_mm_exe_file(current->mm);
> + if (!exe_file)
> + return ERR_PTR(-ENOENT);
> +
> + if (!exe_file->f_inode) {
> + fput(exe_file);
> + return ERR_PTR(-ENOENT);
> + }
> +
> + len = security_inode_getsecurity(exe_file->f_inode, name, data, true);
> + if (len < 0)
> + return ERR_PTR(len);
> +
> + fput(exe_file);
> +
> + return *data;
> +}
> +#endif /* CONFIG_TEE_APPID_SECURITY */
> +
> +#ifdef CONFIG_TEE_APPID_PATH
> +static const char *tee_session_get_application_id(void **data)
> +{
> + struct file *exe_file;
> + char *path;
> +
> + *data = kzalloc(TEE_UUID_NS_NAME_SIZE, GFP_KERNEL);
> + if (!*data)
> + return ERR_PTR(-ENOMEM);
> +
> + exe_file = get_mm_exe_file(current->mm);
> + if (!exe_fil

[PATCH] tee: fix some comment typos in header files

2020-09-19 Thread Elvira Khabirova
struct tee_param: revc -> recv.
TEE_IOC_SUPPL_SEND: typo introduced by copy-pasting, replace invalid
description with description from the according argument struct.

Signed-off-by: Elvira Khabirova 
---
 include/linux/tee_drv.h  | 2 +-
 include/uapi/linux/tee.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
index d074302989dd..61557bc0e29f 100644
--- a/include/linux/tee_drv.h
+++ b/include/linux/tee_drv.h
@@ -85,7 +85,7 @@ struct tee_param {
  * @close_session: close a session
  * @invoke_func:   invoke a trusted function
  * @cancel_req:request cancel of an ongoing invoke or open
- * @supp_revc: called for supplicant to get a command
+ * @supp_recv: called for supplicant to get a command
  * @supp_send: called for supplicant to send a response
  * @shm_register:  register shared memory buffer in TEE
  * @shm_unregister:unregister shared memory buffer in TEE
diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h
index b619f37ee03e..7546be5ed4f8 100644
--- a/include/uapi/linux/tee.h
+++ b/include/uapi/linux/tee.h
@@ -342,7 +342,7 @@ struct tee_iocl_supp_send_arg {
 };
 
 /**
- * TEE_IOC_SUPPL_SEND - Receive a request for a supplicant function
+ * TEE_IOC_SUPPL_SEND - Send a response to a received request
  *
  * Takes a struct tee_ioctl_buf_data which contains a struct
  * tee_iocl_supp_send_arg followed by any array of struct tee_param
-- 
2.28.0



[RFC PATCH] tee: add support for application-based session login methods

2020-09-17 Thread Elvira Khabirova
GP TEE Client API in addition to login methods already supported
in the kernel also defines several application-based methods:
TEEC_LOGIN_APPLICATION, TEEC_LOGIN_USER_APPLICATION, and
TEEC_LOGIN_GROUP_APPLICATION.

It specifies credentials generated for TEEC_LOGIN_APPLICATION as only
depending on the identity of the program, being persistent within one
implementation, across multiple invocations of the application
and across power cycles, enabling them to be used to disambiguate
persistent storage. The exact nature is REE-specific.

As the exact method of generating application identifier strings may
vary between vendors, setups and installations, add two suggested
methods and an exact framework for vendors to extend upon.

Signed-off-by: Elvira Khabirova 
---
 drivers/tee/Kconfig|  29 +
 drivers/tee/tee_core.c | 136 -
 2 files changed, 164 insertions(+), 1 deletion(-)

diff --git a/drivers/tee/Kconfig b/drivers/tee/Kconfig
index e99d840c2511..4cd6e0d2aad5 100644
--- a/drivers/tee/Kconfig
+++ b/drivers/tee/Kconfig
@@ -11,6 +11,35 @@ config TEE
  This implements a generic interface towards a Trusted Execution
  Environment (TEE).
 
+choice
+   prompt "Application ID for client UUID"
+   depends on TEE
+   default TEE_APPID_PATH
+   help
+ This option allows to choose which method will be used to generate
+ application identifiers for client UUID generation when login methods
+ TEE_LOGIN_APPLICATION, TEE_LOGIN_USER_APPLICATION
+ and TEE_LOGIN_GROUP_APPLICATION are used.
+ Please be mindful of the security of each method in your particular
+ installation.
+
+   config TEE_APPID_PATH
+   bool "Path-based application ID"
+   help
+ Use the executable's path as an application ID.
+
+   config TEE_APPID_SECURITY
+   bool "Security extended attribute based application ID"
+   help
+ Use the executable's security extended attribute as an 
application ID.
+endchoice
+
+config TEE_APPID_SECURITY_XATTR
+   string "Security extended attribute to use for application ID"
+   depends on TEE_APPID_SECURITY
+   help
+ Attribute to be used as an application ID (with the security prefix 
removed).
+
 if TEE
 
 menu "TEE drivers"
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
index 64637e09a095..19c965dd212b 100644
--- a/drivers/tee/tee_core.c
+++ b/drivers/tee/tee_core.c
@@ -7,8 +7,10 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -17,11 +19,15 @@
 #include 
 #include "tee_private.h"
 
+#ifdef CONFIG_TEE_APPID_SECURITY
+#include 
+#endif
+
 #define TEE_NUM_DEVICES32
 
 #define TEE_IOCTL_PARAM_SIZE(x) (sizeof(struct tee_param) * (x))
 
-#define TEE_UUID_NS_NAME_SIZE  128
+#define TEE_UUID_NS_NAME_SIZE  PATH_MAX
 
 /*
  * TEE Client UUID name space identifier (UUIDv4)
@@ -125,6 +131,67 @@ static int tee_release(struct inode *inode, struct file 
*filp)
return 0;
 }
 
+#ifdef CONFIG_TEE_APPID_SECURITY
+static const char *tee_session_get_application_id(void **data)
+{
+   struct file *exe_file;
+   const char *name = CONFIG_TEE_APPID_SECURITY_XATTR;
+   int len;
+
+   exe_file = get_mm_exe_file(current->mm);
+   if (!exe_file)
+   return ERR_PTR(-ENOENT);
+
+   if (!exe_file->f_inode) {
+   fput(exe_file);
+   return ERR_PTR(-ENOENT);
+   }
+
+   len = security_inode_getsecurity(exe_file->f_inode, name, data, true);
+   if (len < 0)
+   return ERR_PTR(len);
+
+   fput(exe_file);
+
+   return *data;
+}
+#endif /* CONFIG_TEE_APPID_SECURITY */
+
+#ifdef CONFIG_TEE_APPID_PATH
+static const char *tee_session_get_application_id(void **data)
+{
+   struct file *exe_file;
+   char *path;
+
+   *data = kzalloc(TEE_UUID_NS_NAME_SIZE, GFP_KERNEL);
+   if (!*data)
+   return ERR_PTR(-ENOMEM);
+
+   exe_file = get_mm_exe_file(current->mm);
+   if (!exe_file) {
+   kfree(*data);
+   return ERR_PTR(-ENOENT);
+   }
+
+   path = file_path(exe_file, *data, TEE_UUID_NS_NAME_SIZE);
+   if (IS_ERR(path)) {
+   kfree(*data);
+   return path;
+   }
+
+   fput(exe_file);
+
+   return path;
+}
+#endif /* CONFIG_TEE_APPID_PATH */
+
+#if defined(CONFIG_TEE_APPID_PATH) || defined(CONFIG_TEE_APPID_SECURITY)
+static void tee_session_free_application_id(void *data)
+{
+   kfree(data);
+}
+#endif /* CONFIG_TEE_APPID_PATH || CONFIG_TEE_APPID_SECURITY */
+
 /**
  * uuid_v5() - Calculate UUIDv5
  * @uuid: Resulting UUID
@@ -197,6 +264,8 @@ int tee_session_calc_client_uuid(uuid_t *uuid, u32 
connection_method,
gid_t ns_grp = (gid_t)-1;
kgid_t grp 

Re: [RFC PATCH RESEND v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO

2018-11-26 Thread Elvira Khabirova
On Mon, 26 Nov 2018 15:35:24 +0100
Oleg Nesterov  wrote:

> On 11/25, Elvira Khabirova wrote:
> >
> > Extend PTRACE_GET_SYSCALL_INFO to support PTRACE_EVENT_SECCOMP stops.
> > The information returned is the same as for syscall-enter-stops.  
> 
> Oh, this is not nice ;) there must be a better option, I hope... Plus
> 
> 
> Can't ptrace_get_syscall() check
> 
>   child->exit_code == (PTRACE_EVENT_SECCOMP << 8) | SIGTRAP;
> 
> to detect the PTRACE_EVENT_SECCOMP case?

Nope; looks like exit_code is zeroed after wait().

> Oleg.
> 
> > Signed-off-by: Elvira Khabirova 
> > Signed-off-by: Dmitry V. Levin 
> > ---
> >  include/linux/ptrace.h| 1 +
> >  include/linux/sched.h | 1 +
> >  include/linux/tracehook.h | 1 +
> >  kernel/ptrace.c   | 7 +--
> >  4 files changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
> > index 6c2ffed907f5..a993d0fde865 100644
> > --- a/include/linux/ptrace.h
> > +++ b/include/linux/ptrace.h
> > @@ -166,6 +166,7 @@ static inline void ptrace_event(int event, unsigned 
> > long message)
> >  {
> > if (unlikely(ptrace_event_enabled(current, event))) {
> > current->ptrace_message = message;
> > +   current->ptrace_event = event;
> > ptrace_notify((event << 8) | SIGTRAP);
> > } else if (event == PTRACE_EVENT_EXEC) {
> > /* legacy EXEC report via SIGTRAP */
> > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > index a51c13c2b1a0..86215fb654d6 100644
> > --- a/include/linux/sched.h
> > +++ b/include/linux/sched.h
> > @@ -964,6 +964,7 @@ struct task_struct {
> >  
> > /* Ptrace state: */
> > unsigned long   ptrace_message;
> > +   int ptrace_event;
> > kernel_siginfo_t*last_siginfo;
> >  
> > struct task_io_accounting   ioac;
> > diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
> > index 633a83fe7051..5d2e5aa07a5c 100644
> > --- a/include/linux/tracehook.h
> > +++ b/include/linux/tracehook.h
> > @@ -66,6 +66,7 @@ static inline int ptrace_report_syscall(struct pt_regs 
> > *regs,
> > return 0;
> >  
> > current->ptrace_message = message;
> > +   current->ptrace_event = 0;
> > ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0));
> >  
> > /*
> > diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> > index 92c47cd5ad84..74a37e74c7f1 100644
> > --- a/kernel/ptrace.c
> > +++ b/kernel/ptrace.c
> > @@ -904,7 +904,9 @@ static int ptrace_get_syscall(struct task_struct *child,
> > unsigned long actual_size;
> > unsigned long write_size;
> >  
> > -   if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) {
> > +   if ((child->ptrace_event == 0 &&
> > +child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) ||
> > +   child->ptrace_event == PTRACE_EVENT_SECCOMP) {
> > int i;
> >  
> > info.op = PTRACE_SYSCALL_INFO_ENTRY;
> > @@ -917,7 +919,8 @@ static int ptrace_get_syscall(struct task_struct *child,
> > for (i = 0; i < ARRAY_SIZE(args); i++)
> > info.entry.args[i] = args[i];
> > actual_size = offsetofend(struct ptrace_syscall_info, entry);
> > -   } else if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) {
> > +   } else if (child->ptrace_event == 0 &&
> > +  child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) {
> > info.op = PTRACE_SYSCALL_INFO_EXIT;
> > info.arch = syscall_get_arch(child);
> > info.exit.rval = syscall_get_error(child, regs);
> > -- 
> > 2.19.1
> >   
> 



Re: [RFC PATCH RESEND v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO

2018-11-26 Thread Elvira Khabirova
On Mon, 26 Nov 2018 15:35:24 +0100
Oleg Nesterov  wrote:

> On 11/25, Elvira Khabirova wrote:
> >
> > Extend PTRACE_GET_SYSCALL_INFO to support PTRACE_EVENT_SECCOMP stops.
> > The information returned is the same as for syscall-enter-stops.  
> 
> Oh, this is not nice ;) there must be a better option, I hope... Plus
> 
> 
> Can't ptrace_get_syscall() check
> 
>   child->exit_code == (PTRACE_EVENT_SECCOMP << 8) | SIGTRAP;
> 
> to detect the PTRACE_EVENT_SECCOMP case?

Nope; looks like exit_code is zeroed after wait().

> Oleg.
> 
> > Signed-off-by: Elvira Khabirova 
> > Signed-off-by: Dmitry V. Levin 
> > ---
> >  include/linux/ptrace.h| 1 +
> >  include/linux/sched.h | 1 +
> >  include/linux/tracehook.h | 1 +
> >  kernel/ptrace.c   | 7 +--
> >  4 files changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
> > index 6c2ffed907f5..a993d0fde865 100644
> > --- a/include/linux/ptrace.h
> > +++ b/include/linux/ptrace.h
> > @@ -166,6 +166,7 @@ static inline void ptrace_event(int event, unsigned 
> > long message)
> >  {
> > if (unlikely(ptrace_event_enabled(current, event))) {
> > current->ptrace_message = message;
> > +   current->ptrace_event = event;
> > ptrace_notify((event << 8) | SIGTRAP);
> > } else if (event == PTRACE_EVENT_EXEC) {
> > /* legacy EXEC report via SIGTRAP */
> > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > index a51c13c2b1a0..86215fb654d6 100644
> > --- a/include/linux/sched.h
> > +++ b/include/linux/sched.h
> > @@ -964,6 +964,7 @@ struct task_struct {
> >  
> > /* Ptrace state: */
> > unsigned long   ptrace_message;
> > +   int ptrace_event;
> > kernel_siginfo_t*last_siginfo;
> >  
> > struct task_io_accounting   ioac;
> > diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
> > index 633a83fe7051..5d2e5aa07a5c 100644
> > --- a/include/linux/tracehook.h
> > +++ b/include/linux/tracehook.h
> > @@ -66,6 +66,7 @@ static inline int ptrace_report_syscall(struct pt_regs 
> > *regs,
> > return 0;
> >  
> > current->ptrace_message = message;
> > +   current->ptrace_event = 0;
> > ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0));
> >  
> > /*
> > diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> > index 92c47cd5ad84..74a37e74c7f1 100644
> > --- a/kernel/ptrace.c
> > +++ b/kernel/ptrace.c
> > @@ -904,7 +904,9 @@ static int ptrace_get_syscall(struct task_struct *child,
> > unsigned long actual_size;
> > unsigned long write_size;
> >  
> > -   if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) {
> > +   if ((child->ptrace_event == 0 &&
> > +child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) ||
> > +   child->ptrace_event == PTRACE_EVENT_SECCOMP) {
> > int i;
> >  
> > info.op = PTRACE_SYSCALL_INFO_ENTRY;
> > @@ -917,7 +919,8 @@ static int ptrace_get_syscall(struct task_struct *child,
> > for (i = 0; i < ARRAY_SIZE(args); i++)
> > info.entry.args[i] = args[i];
> > actual_size = offsetofend(struct ptrace_syscall_info, entry);
> > -   } else if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) {
> > +   } else if (child->ptrace_event == 0 &&
> > +  child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) {
> > info.op = PTRACE_SYSCALL_INFO_EXIT;
> > info.arch = syscall_get_arch(child);
> > info.exit.rval = syscall_get_error(child, regs);
> > -- 
> > 2.19.1
> >   
> 



Re: [PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message

2018-11-26 Thread Elvira Khabirova
On Mon, 26 Nov 2018 15:56:43 +0100
Oleg Nesterov  wrote:

> On 11/25, Elvira Khabirova wrote:
> >
> > + * These values are stored in task->ptrace_message by 
> > tracehook_report_syscall_*
> > + * to describe current syscall-stop.
> > + *
> > + * Values for these constants are chosen so that they do not appear
> > + * in task->ptrace_message by other means.
> > + */
> > +#define PTRACE_EVENTMSG_SYSCALL_ENTRY  0x8000U
> > +#define PTRACE_EVENTMSG_SYSCALL_EXIT   0x9000U  
> 
> Stupid question, why not
> 
>   #define PTRACE_EVENT_SYSCALL_ENTRY  8
>   #define PTRACE_EVENT_SYSCALL_EXIT   9
> 
> right after other PTRACE_EVENT_* constants?

I thought about adding new events for syscall {entry,exit}.
For tracers, using new events means setting new options and checking
for new values after waitpid(). They will also have to switch from using
PTRACE_SYSCALL to PTRACE_CONT.
Right now (with this version of the patch) tracers can use
PTRACE_GETEVENTMSG without doing any additional configuration.
More importantly, adding these events would require much more complex
modifications of kernel code than this patch does.
The only benefit I see from adding these events instead of letting
syscall-stops put a value in ptrace_message is an ability to subscribe
to syscall entries, but not to exits, and vice-versa, and I don't think
it is worth it.


Re: [PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message

2018-11-26 Thread Elvira Khabirova
On Mon, 26 Nov 2018 15:56:43 +0100
Oleg Nesterov  wrote:

> On 11/25, Elvira Khabirova wrote:
> >
> > + * These values are stored in task->ptrace_message by 
> > tracehook_report_syscall_*
> > + * to describe current syscall-stop.
> > + *
> > + * Values for these constants are chosen so that they do not appear
> > + * in task->ptrace_message by other means.
> > + */
> > +#define PTRACE_EVENTMSG_SYSCALL_ENTRY  0x8000U
> > +#define PTRACE_EVENTMSG_SYSCALL_EXIT   0x9000U  
> 
> Stupid question, why not
> 
>   #define PTRACE_EVENT_SYSCALL_ENTRY  8
>   #define PTRACE_EVENT_SYSCALL_EXIT   9
> 
> right after other PTRACE_EVENT_* constants?

I thought about adding new events for syscall {entry,exit}.
For tracers, using new events means setting new options and checking
for new values after waitpid(). They will also have to switch from using
PTRACE_SYSCALL to PTRACE_CONT.
Right now (with this version of the patch) tracers can use
PTRACE_GETEVENTMSG without doing any additional configuration.
More importantly, adding these events would require much more complex
modifications of kernel code than this patch does.
The only benefit I see from adding these events instead of letting
syscall-stops put a value in ptrace_message is an ability to subscribe
to syscall entries, but not to exits, and vice-versa, and I don't think
it is worth it.


[RFC PATCH RESEND v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO

2018-11-24 Thread Elvira Khabirova
Extend PTRACE_GET_SYSCALL_INFO to support PTRACE_EVENT_SECCOMP stops.
The information returned is the same as for syscall-enter-stops.

Signed-off-by: Elvira Khabirova 
Signed-off-by: Dmitry V. Levin 
---
 include/linux/ptrace.h| 1 +
 include/linux/sched.h | 1 +
 include/linux/tracehook.h | 1 +
 kernel/ptrace.c   | 7 +--
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index 6c2ffed907f5..a993d0fde865 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -166,6 +166,7 @@ static inline void ptrace_event(int event, unsigned long 
message)
 {
if (unlikely(ptrace_event_enabled(current, event))) {
current->ptrace_message = message;
+   current->ptrace_event = event;
ptrace_notify((event << 8) | SIGTRAP);
} else if (event == PTRACE_EVENT_EXEC) {
/* legacy EXEC report via SIGTRAP */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index a51c13c2b1a0..86215fb654d6 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -964,6 +964,7 @@ struct task_struct {
 
/* Ptrace state: */
unsigned long   ptrace_message;
+   int ptrace_event;
kernel_siginfo_t*last_siginfo;
 
struct task_io_accounting   ioac;
diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
index 633a83fe7051..5d2e5aa07a5c 100644
--- a/include/linux/tracehook.h
+++ b/include/linux/tracehook.h
@@ -66,6 +66,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs,
return 0;
 
current->ptrace_message = message;
+   current->ptrace_event = 0;
ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0));
 
/*
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 92c47cd5ad84..74a37e74c7f1 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -904,7 +904,9 @@ static int ptrace_get_syscall(struct task_struct *child,
unsigned long actual_size;
unsigned long write_size;
 
-   if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) {
+   if ((child->ptrace_event == 0 &&
+child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) ||
+   child->ptrace_event == PTRACE_EVENT_SECCOMP) {
int i;
 
info.op = PTRACE_SYSCALL_INFO_ENTRY;
@@ -917,7 +919,8 @@ static int ptrace_get_syscall(struct task_struct *child,
for (i = 0; i < ARRAY_SIZE(args); i++)
info.entry.args[i] = args[i];
actual_size = offsetofend(struct ptrace_syscall_info, entry);
-   } else if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) {
+   } else if (child->ptrace_event == 0 &&
+  child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) {
info.op = PTRACE_SYSCALL_INFO_EXIT;
info.arch = syscall_get_arch(child);
info.exit.rval = syscall_get_error(child, regs);
-- 
2.19.1



[RFC PATCH RESEND v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO

2018-11-24 Thread Elvira Khabirova
Extend PTRACE_GET_SYSCALL_INFO to support PTRACE_EVENT_SECCOMP stops.
The information returned is the same as for syscall-enter-stops.

Signed-off-by: Elvira Khabirova 
Signed-off-by: Dmitry V. Levin 
---
 include/linux/ptrace.h| 1 +
 include/linux/sched.h | 1 +
 include/linux/tracehook.h | 1 +
 kernel/ptrace.c   | 7 +--
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index 6c2ffed907f5..a993d0fde865 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -166,6 +166,7 @@ static inline void ptrace_event(int event, unsigned long 
message)
 {
if (unlikely(ptrace_event_enabled(current, event))) {
current->ptrace_message = message;
+   current->ptrace_event = event;
ptrace_notify((event << 8) | SIGTRAP);
} else if (event == PTRACE_EVENT_EXEC) {
/* legacy EXEC report via SIGTRAP */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index a51c13c2b1a0..86215fb654d6 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -964,6 +964,7 @@ struct task_struct {
 
/* Ptrace state: */
unsigned long   ptrace_message;
+   int ptrace_event;
kernel_siginfo_t*last_siginfo;
 
struct task_io_accounting   ioac;
diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
index 633a83fe7051..5d2e5aa07a5c 100644
--- a/include/linux/tracehook.h
+++ b/include/linux/tracehook.h
@@ -66,6 +66,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs,
return 0;
 
current->ptrace_message = message;
+   current->ptrace_event = 0;
ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0));
 
/*
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 92c47cd5ad84..74a37e74c7f1 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -904,7 +904,9 @@ static int ptrace_get_syscall(struct task_struct *child,
unsigned long actual_size;
unsigned long write_size;
 
-   if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) {
+   if ((child->ptrace_event == 0 &&
+child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) ||
+   child->ptrace_event == PTRACE_EVENT_SECCOMP) {
int i;
 
info.op = PTRACE_SYSCALL_INFO_ENTRY;
@@ -917,7 +919,8 @@ static int ptrace_get_syscall(struct task_struct *child,
for (i = 0; i < ARRAY_SIZE(args); i++)
info.entry.args[i] = args[i];
actual_size = offsetofend(struct ptrace_syscall_info, entry);
-   } else if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) {
+   } else if (child->ptrace_event == 0 &&
+  child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) {
info.op = PTRACE_SYSCALL_INFO_EXIT;
info.arch = syscall_get_arch(child);
info.exit.rval = syscall_get_error(child, regs);
-- 
2.19.1



[PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message

2018-11-24 Thread Elvira Khabirova
Define two constants, PTRACE_EVENTMSG_SYSCALL_ENTRY and
PTRACE_EVENTMSG_SYSCALL_EXIT, and place them in ptrace_message
for the duration of syscall-stops.
This way ptracers can distinguish syscall-enter-stops
from syscall-exit-stops using PTRACE_GETEVENTMSG request.

Signed-off-by: Elvira Khabirova 
Signed-off-by: Dmitry V. Levin 
---
 include/linux/tracehook.h   |  9 ++---
 include/uapi/linux/ptrace.h | 10 ++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
index 40b0b4c1bf7b..633a83fe7051 100644
--- a/include/linux/tracehook.h
+++ b/include/linux/tracehook.h
@@ -57,13 +57,15 @@ struct linux_binprm;
 /*
  * ptrace report for syscall entry and exit looks identical.
  */
-static inline int ptrace_report_syscall(struct pt_regs *regs)
+static inline int ptrace_report_syscall(struct pt_regs *regs,
+   unsigned long message)
 {
int ptrace = current->ptrace;
 
if (!(ptrace & PT_PTRACED))
return 0;
 
+   current->ptrace_message = message;
ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0));
 
/*
@@ -76,6 +78,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs)
current->exit_code = 0;
}
 
+   current->ptrace_message = 0;
return fatal_signal_pending(current);
 }
 
@@ -101,7 +104,7 @@ static inline int ptrace_report_syscall(struct pt_regs 
*regs)
 static inline __must_check int tracehook_report_syscall_entry(
struct pt_regs *regs)
 {
-   return ptrace_report_syscall(regs);
+   return ptrace_report_syscall(regs, PTRACE_EVENTMSG_SYSCALL_ENTRY);
 }
 
 /**
@@ -126,7 +129,7 @@ static inline void tracehook_report_syscall_exit(struct 
pt_regs *regs, int step)
if (step)
user_single_step_report(regs);
else
-   ptrace_report_syscall(regs);
+   ptrace_report_syscall(regs, PTRACE_EVENTMSG_SYSCALL_EXIT);
 }
 
 /**
diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
index d5a1b8a492b9..cb138902d042 100644
--- a/include/uapi/linux/ptrace.h
+++ b/include/uapi/linux/ptrace.h
@@ -104,6 +104,16 @@ struct seccomp_metadata {
 #define PTRACE_O_MASK  (\
0x00ff | PTRACE_O_EXITKILL | PTRACE_O_SUSPEND_SECCOMP)
 
+/*
+ * These values are stored in task->ptrace_message by 
tracehook_report_syscall_*
+ * to describe current syscall-stop.
+ *
+ * Values for these constants are chosen so that they do not appear
+ * in task->ptrace_message by other means.
+ */
+#define PTRACE_EVENTMSG_SYSCALL_ENTRY  0x8000U
+#define PTRACE_EVENTMSG_SYSCALL_EXIT   0x9000U
+
 #include 
 
 
-- 
2.19.1



[PATCH RESEND v3 2/3] ptrace: add PTRACE_GET_SYSCALL_INFO request

2018-11-24 Thread Elvira Khabirova
PTRACE_GET_SYSCALL_INFO lets ptracer obtain details of the syscall
the tracee is blocked in.  The request succeeds when the tracee is in a
syscall-enter-stop or syscall-exit-stop, and fails with -EINVAL otherwise.
A subsequent change may extend PTRACE_GET_SYSCALL_INFO for the case
of PTRACE_EVENT_SECCOMP stop.

There are two reasons for a special syscall-related ptrace request.

Firstly, with the current ptrace API there are cases when ptracer cannot
retrieve necessary information about syscalls.  Some examples include:
* The notorious int-0x80-from-64-bit-task issue.  See [1] for details.
In short, if a 64-bit task performs a syscall through int 0x80, its tracer
has no reliable means to find out that the syscall was, in fact,
a compat syscall, and misidentifies it.
* Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
Common practice is to keep track of the sequence of ptrace-stops in order
not to mix the two syscall-stops up.  But it is not as simple as it looks;
for example, strace had a (just recently fixed) long-standing bug where
attaching strace to a tracee that is performing the execve system call
led to the tracer identifying the following syscall-exit-stop as
syscall-enter-stop, which messed up all the state tracking.
* Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
and process_vm_readv become unavailable when the process dumpable flag
is cleared.  On such architectures as ia64 this results in all syscall
arguments being unavailable.

Secondly, ptracers also have to support a lot of arch-specific code for
obtaining information about the tracee.  For some architectures, this
requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
argument and return value.

ptrace(2) man page:

long ptrace(enum __ptrace_request request, pid_t pid,
void *addr, void *data);
...
PTRACE_GET_SYSCALL_INFO
   Retrieve information about the syscall that caused the stop.
   The information is placed into the buffer pointed by "data"
   argument, which should be a pointer to a buffer of type
   "struct ptrace_syscall_info".
   The "addr" argument contains the size of the buffer pointed to
   by "data" (i.e., sizeof(struct ptrace_syscall_info)).
   The return value contains the number of bytes available
   to be written by the kernel.
   If the size of data to be written by the kernel exceeds the size
   specified by "addr" argument, the output is truncated.
   This operation fails with EINVAL if the tracee is not
   in a syscall-enter-stop, a syscall-exit-stop, or
   a PTRACE_EVENT_SECCOMP stop.

Signed-off-by: Elvira Khabirova 
Signed-off-by: Dmitry V. Levin 
---
 include/uapi/linux/ptrace.h | 24 ++
 kernel/ptrace.c | 50 +
 2 files changed, 74 insertions(+)

diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
index cb138902d042..49b0b1b41943 100644
--- a/include/uapi/linux/ptrace.h
+++ b/include/uapi/linux/ptrace.h
@@ -73,6 +73,30 @@ struct seccomp_metadata {
__u64 flags;/* Output: filter's flags */
 };
 
+#define PTRACE_GET_SYSCALL_INFO0x420f
+#define PTRACE_SYSCALL_INFO_ENTRY  0
+#define PTRACE_SYSCALL_INFO_EXIT   1
+
+struct ptrace_syscall_info {
+   __u8 op;/* PTRACE_SYSCALL_INFO_* */
+   __u8 __pad0[3];
+   __u32 arch;
+   union {
+   struct {
+   __u64 nr;
+   __u64 instruction_pointer;
+   __u64 stack_pointer;
+   __u64 frame_pointer;
+   __u64 args[6];
+   } entry;
+   struct {
+   __s64 rval;
+   __u8 is_error;
+   __u8 __pad1[7];
+   } exit;
+   };
+};
+
 /* Read signals from a shared (process wide) queue */
 #define PTRACE_PEEKSIGINFO_SHARED  (1 << 0)
 
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 80b34dffdfb9..92c47cd5ad84 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -30,6 +30,10 @@
 #include 
 #include 
 
+#ifdef CONFIG_HAVE_ARCH_TRACEHOOK
+#include/* For syscall_get_* */
+#endif
+
 /*
  * Access another process' address space via ptrace.
  * Source/target buffer must be kernel space,
@@ -890,6 +894,46 @@ static int ptrace_regset(struct task_struct *task, int 
req, unsigned int type,
 EXPORT_SYMBOL_GPL(task_user_regset_view);
 #endif
 
+#ifdef CONFIG_HAVE_ARCH_TRACEHOOK
+static int ptrace_get_syscall(struct task_struct *child,
+ unsigned long user_size, void __user *datavp)
+{
+   struct ptrace_syscall_info info;
+   struct pt_regs *regs = task_pt_regs(child);
+   unsigned long args[ARRAY_SIZE(info.entry.args)];
+

[PATCH RESEND v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message

2018-11-24 Thread Elvira Khabirova
Define two constants, PTRACE_EVENTMSG_SYSCALL_ENTRY and
PTRACE_EVENTMSG_SYSCALL_EXIT, and place them in ptrace_message
for the duration of syscall-stops.
This way ptracers can distinguish syscall-enter-stops
from syscall-exit-stops using PTRACE_GETEVENTMSG request.

Signed-off-by: Elvira Khabirova 
Signed-off-by: Dmitry V. Levin 
---
 include/linux/tracehook.h   |  9 ++---
 include/uapi/linux/ptrace.h | 10 ++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
index 40b0b4c1bf7b..633a83fe7051 100644
--- a/include/linux/tracehook.h
+++ b/include/linux/tracehook.h
@@ -57,13 +57,15 @@ struct linux_binprm;
 /*
  * ptrace report for syscall entry and exit looks identical.
  */
-static inline int ptrace_report_syscall(struct pt_regs *regs)
+static inline int ptrace_report_syscall(struct pt_regs *regs,
+   unsigned long message)
 {
int ptrace = current->ptrace;
 
if (!(ptrace & PT_PTRACED))
return 0;
 
+   current->ptrace_message = message;
ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0));
 
/*
@@ -76,6 +78,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs)
current->exit_code = 0;
}
 
+   current->ptrace_message = 0;
return fatal_signal_pending(current);
 }
 
@@ -101,7 +104,7 @@ static inline int ptrace_report_syscall(struct pt_regs 
*regs)
 static inline __must_check int tracehook_report_syscall_entry(
struct pt_regs *regs)
 {
-   return ptrace_report_syscall(regs);
+   return ptrace_report_syscall(regs, PTRACE_EVENTMSG_SYSCALL_ENTRY);
 }
 
 /**
@@ -126,7 +129,7 @@ static inline void tracehook_report_syscall_exit(struct 
pt_regs *regs, int step)
if (step)
user_single_step_report(regs);
else
-   ptrace_report_syscall(regs);
+   ptrace_report_syscall(regs, PTRACE_EVENTMSG_SYSCALL_EXIT);
 }
 
 /**
diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
index d5a1b8a492b9..cb138902d042 100644
--- a/include/uapi/linux/ptrace.h
+++ b/include/uapi/linux/ptrace.h
@@ -104,6 +104,16 @@ struct seccomp_metadata {
 #define PTRACE_O_MASK  (\
0x00ff | PTRACE_O_EXITKILL | PTRACE_O_SUSPEND_SECCOMP)
 
+/*
+ * These values are stored in task->ptrace_message by 
tracehook_report_syscall_*
+ * to describe current syscall-stop.
+ *
+ * Values for these constants are chosen so that they do not appear
+ * in task->ptrace_message by other means.
+ */
+#define PTRACE_EVENTMSG_SYSCALL_ENTRY  0x8000U
+#define PTRACE_EVENTMSG_SYSCALL_EXIT   0x9000U
+
 #include 
 
 
-- 
2.19.1



[PATCH RESEND v3 2/3] ptrace: add PTRACE_GET_SYSCALL_INFO request

2018-11-24 Thread Elvira Khabirova
PTRACE_GET_SYSCALL_INFO lets ptracer obtain details of the syscall
the tracee is blocked in.  The request succeeds when the tracee is in a
syscall-enter-stop or syscall-exit-stop, and fails with -EINVAL otherwise.
A subsequent change may extend PTRACE_GET_SYSCALL_INFO for the case
of PTRACE_EVENT_SECCOMP stop.

There are two reasons for a special syscall-related ptrace request.

Firstly, with the current ptrace API there are cases when ptracer cannot
retrieve necessary information about syscalls.  Some examples include:
* The notorious int-0x80-from-64-bit-task issue.  See [1] for details.
In short, if a 64-bit task performs a syscall through int 0x80, its tracer
has no reliable means to find out that the syscall was, in fact,
a compat syscall, and misidentifies it.
* Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
Common practice is to keep track of the sequence of ptrace-stops in order
not to mix the two syscall-stops up.  But it is not as simple as it looks;
for example, strace had a (just recently fixed) long-standing bug where
attaching strace to a tracee that is performing the execve system call
led to the tracer identifying the following syscall-exit-stop as
syscall-enter-stop, which messed up all the state tracking.
* Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
and process_vm_readv become unavailable when the process dumpable flag
is cleared.  On such architectures as ia64 this results in all syscall
arguments being unavailable.

Secondly, ptracers also have to support a lot of arch-specific code for
obtaining information about the tracee.  For some architectures, this
requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
argument and return value.

ptrace(2) man page:

long ptrace(enum __ptrace_request request, pid_t pid,
void *addr, void *data);
...
PTRACE_GET_SYSCALL_INFO
   Retrieve information about the syscall that caused the stop.
   The information is placed into the buffer pointed by "data"
   argument, which should be a pointer to a buffer of type
   "struct ptrace_syscall_info".
   The "addr" argument contains the size of the buffer pointed to
   by "data" (i.e., sizeof(struct ptrace_syscall_info)).
   The return value contains the number of bytes available
   to be written by the kernel.
   If the size of data to be written by the kernel exceeds the size
   specified by "addr" argument, the output is truncated.
   This operation fails with EINVAL if the tracee is not
   in a syscall-enter-stop, a syscall-exit-stop, or
   a PTRACE_EVENT_SECCOMP stop.

Signed-off-by: Elvira Khabirova 
Signed-off-by: Dmitry V. Levin 
---
 include/uapi/linux/ptrace.h | 24 ++
 kernel/ptrace.c | 50 +
 2 files changed, 74 insertions(+)

diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
index cb138902d042..49b0b1b41943 100644
--- a/include/uapi/linux/ptrace.h
+++ b/include/uapi/linux/ptrace.h
@@ -73,6 +73,30 @@ struct seccomp_metadata {
__u64 flags;/* Output: filter's flags */
 };
 
+#define PTRACE_GET_SYSCALL_INFO0x420f
+#define PTRACE_SYSCALL_INFO_ENTRY  0
+#define PTRACE_SYSCALL_INFO_EXIT   1
+
+struct ptrace_syscall_info {
+   __u8 op;/* PTRACE_SYSCALL_INFO_* */
+   __u8 __pad0[3];
+   __u32 arch;
+   union {
+   struct {
+   __u64 nr;
+   __u64 instruction_pointer;
+   __u64 stack_pointer;
+   __u64 frame_pointer;
+   __u64 args[6];
+   } entry;
+   struct {
+   __s64 rval;
+   __u8 is_error;
+   __u8 __pad1[7];
+   } exit;
+   };
+};
+
 /* Read signals from a shared (process wide) queue */
 #define PTRACE_PEEKSIGINFO_SHARED  (1 << 0)
 
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 80b34dffdfb9..92c47cd5ad84 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -30,6 +30,10 @@
 #include 
 #include 
 
+#ifdef CONFIG_HAVE_ARCH_TRACEHOOK
+#include/* For syscall_get_* */
+#endif
+
 /*
  * Access another process' address space via ptrace.
  * Source/target buffer must be kernel space,
@@ -890,6 +894,46 @@ static int ptrace_regset(struct task_struct *task, int 
req, unsigned int type,
 EXPORT_SYMBOL_GPL(task_user_regset_view);
 #endif
 
+#ifdef CONFIG_HAVE_ARCH_TRACEHOOK
+static int ptrace_get_syscall(struct task_struct *child,
+ unsigned long user_size, void __user *datavp)
+{
+   struct ptrace_syscall_info info;
+   struct pt_regs *regs = task_pt_regs(child);
+   unsigned long args[ARRAY_SIZE(info.entry.args)];
+

[PATCH RESEND v3 0/3] ptrace: add PTRACE_GET_SYSCALL_INFO request

2018-11-24 Thread Elvira Khabirova
Resent with linux-api@ Cc'ed.

PTRACE_GET_SYSCALL_INFO lets ptracer obtain details of the syscall
the tracee is blocked in.  The request succeeds when the tracee is in a
syscall-enter-stop, syscall-exit-stop or PTRACE_EVENT_SECCOMP stop,
and fails with -EINVAL otherwise.

There are two reasons for a special syscall-related ptrace request.

Firstly, with the current ptrace API there are cases when ptracer cannot
retrieve necessary information about syscalls.  Some examples include:
* The notorious int-0x80-from-64-bit-task issue.  See [1] for details.
In short, if a 64-bit task performs a syscall through int 0x80, its tracer
has no reliable means to find out that the syscall was, in fact,
a compat syscall, and misidentifies it.
* Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
Common practice is to keep track of the sequence of ptrace-stops in order
not to mix the two syscall-stops up.  But it is not as simple as it looks;
for example, strace had a (just recently fixed) long-standing bug where
attaching strace to a tracee that is performing the execve system call
led to the tracer identifying the following syscall-exit-stop as
syscall-enter-stop, which messed up all the state tracking.
* Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
and process_vm_readv become unavailable when the process dumpable flag
is cleared.  On such architectures as ia64 this results in all syscall
arguments being unavailable.

Secondly, ptracers also have to support a lot of arch-specific code for
obtaining information about the tracee.  For some architectures, this
requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
argument and return value.

PTRACE_GET_SYSCALL_INFO returns the following structure:

struct ptrace_syscall_info {
__u8 op;/* PTRACE_SYSCALL_INFO_* */
__u8 __pad0[3];
__u32 arch;
union {
struct {
__u64 nr;
__u64 instruction_pointer;
__u64 stack_pointer;
__u64 frame_pointer;
__u64 args[6];
} entry;
struct {
__s64 rval;
__u8 is_error;
__u8 __pad1[7];
} exit;
};
};

The structure was chosen according to [2], except for the following
changes:
* arch is returned unconditionally to aid with tracing system calls such as
execve();
* the type of nr field was changed from int to __u64 because syscall
numbers are, as a practical matter, 64 bits;
* stack_pointer and frame_pointer fields were added along with
instruction_pointer field since they are readily available and can save
the tracer from extra PTRACE_GETREGSET calls;
* a boolean is_error field was added along with rval field, this way
the tracer can more reliably distinguish a return value
from an error value.

This changeset should be applied on top of [3] and [4].

[1] 
https://lore.kernel.org/lkml/ca+55afzcsvmddj9lh_gdbz1ozhyem6zrgpbdajnywm2lf_e...@mail.gmail.com/
[2] 
https://lore.kernel.org/lkml/caobl_7gm0n80n7j_dfw_eqyflyzq+sf4y2avsccv88tb3aw...@mail.gmail.com/
[3] https://lore.kernel.org/lkml/20181119210139.ga8...@altlinux.org/
[4] https://lore.kernel.org/lkml/20181120001128.ga11...@altlinux.org/

v3: Split into three changes.
Change struct ptrace_syscall_info.
Support PTRACE_EVENT_SECCOMP by adding ptrace_event to task_struct.
Add proper defines for ptrace_syscall_info.op values.
Rename PT_SYSCALL_IS_ENTERING and PT_SYSCALL_IS_EXITING to
PTRACE_EVENTMSG_SYSCALL_ENTRY and PTRACE_EVENTMSG_SYSCALL_EXIT and move
them to uapi.

Elvira Khabirova (3):
  ptrace: pass type of a syscall-stop in ptrace_message
  ptrace: add PTRACE_GET_SYSCALL_INFO request
  ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO

 include/linux/ptrace.h  |  1 +
 include/linux/sched.h   |  1 +
 include/linux/tracehook.h   | 10 ---
 include/uapi/linux/ptrace.h | 34 
 kernel/ptrace.c | 53 +
 5 files changed, 96 insertions(+), 3 deletions(-)

-- 
2.19.1



[PATCH RESEND v3 0/3] ptrace: add PTRACE_GET_SYSCALL_INFO request

2018-11-24 Thread Elvira Khabirova
Resent with linux-api@ Cc'ed.

PTRACE_GET_SYSCALL_INFO lets ptracer obtain details of the syscall
the tracee is blocked in.  The request succeeds when the tracee is in a
syscall-enter-stop, syscall-exit-stop or PTRACE_EVENT_SECCOMP stop,
and fails with -EINVAL otherwise.

There are two reasons for a special syscall-related ptrace request.

Firstly, with the current ptrace API there are cases when ptracer cannot
retrieve necessary information about syscalls.  Some examples include:
* The notorious int-0x80-from-64-bit-task issue.  See [1] for details.
In short, if a 64-bit task performs a syscall through int 0x80, its tracer
has no reliable means to find out that the syscall was, in fact,
a compat syscall, and misidentifies it.
* Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
Common practice is to keep track of the sequence of ptrace-stops in order
not to mix the two syscall-stops up.  But it is not as simple as it looks;
for example, strace had a (just recently fixed) long-standing bug where
attaching strace to a tracee that is performing the execve system call
led to the tracer identifying the following syscall-exit-stop as
syscall-enter-stop, which messed up all the state tracking.
* Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
and process_vm_readv become unavailable when the process dumpable flag
is cleared.  On such architectures as ia64 this results in all syscall
arguments being unavailable.

Secondly, ptracers also have to support a lot of arch-specific code for
obtaining information about the tracee.  For some architectures, this
requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
argument and return value.

PTRACE_GET_SYSCALL_INFO returns the following structure:

struct ptrace_syscall_info {
__u8 op;/* PTRACE_SYSCALL_INFO_* */
__u8 __pad0[3];
__u32 arch;
union {
struct {
__u64 nr;
__u64 instruction_pointer;
__u64 stack_pointer;
__u64 frame_pointer;
__u64 args[6];
} entry;
struct {
__s64 rval;
__u8 is_error;
__u8 __pad1[7];
} exit;
};
};

The structure was chosen according to [2], except for the following
changes:
* arch is returned unconditionally to aid with tracing system calls such as
execve();
* the type of nr field was changed from int to __u64 because syscall
numbers are, as a practical matter, 64 bits;
* stack_pointer and frame_pointer fields were added along with
instruction_pointer field since they are readily available and can save
the tracer from extra PTRACE_GETREGSET calls;
* a boolean is_error field was added along with rval field, this way
the tracer can more reliably distinguish a return value
from an error value.

This changeset should be applied on top of [3] and [4].

[1] 
https://lore.kernel.org/lkml/ca+55afzcsvmddj9lh_gdbz1ozhyem6zrgpbdajnywm2lf_e...@mail.gmail.com/
[2] 
https://lore.kernel.org/lkml/caobl_7gm0n80n7j_dfw_eqyflyzq+sf4y2avsccv88tb3aw...@mail.gmail.com/
[3] https://lore.kernel.org/lkml/20181119210139.ga8...@altlinux.org/
[4] https://lore.kernel.org/lkml/20181120001128.ga11...@altlinux.org/

v3: Split into three changes.
Change struct ptrace_syscall_info.
Support PTRACE_EVENT_SECCOMP by adding ptrace_event to task_struct.
Add proper defines for ptrace_syscall_info.op values.
Rename PT_SYSCALL_IS_ENTERING and PT_SYSCALL_IS_EXITING to
PTRACE_EVENTMSG_SYSCALL_ENTRY and PTRACE_EVENTMSG_SYSCALL_EXIT and move
them to uapi.

Elvira Khabirova (3):
  ptrace: pass type of a syscall-stop in ptrace_message
  ptrace: add PTRACE_GET_SYSCALL_INFO request
  ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO

 include/linux/ptrace.h  |  1 +
 include/linux/sched.h   |  1 +
 include/linux/tracehook.h   | 10 ---
 include/uapi/linux/ptrace.h | 34 
 kernel/ptrace.c | 53 +
 5 files changed, 96 insertions(+), 3 deletions(-)

-- 
2.19.1



[RFC PATCH v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO

2018-11-24 Thread Elvira Khabirova
Extend PTRACE_GET_SYSCALL_INFO to support PTRACE_EVENT_SECCOMP stops.
The information returned is the same as for syscall-enter-stops.

Signed-off-by: Elvira Khabirova 
Signed-off-by: Dmitry V. Levin 
---
 include/linux/ptrace.h| 1 +
 include/linux/sched.h | 1 +
 include/linux/tracehook.h | 1 +
 kernel/ptrace.c   | 7 +--
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index 6c2ffed907f5..a993d0fde865 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -166,6 +166,7 @@ static inline void ptrace_event(int event, unsigned long 
message)
 {
if (unlikely(ptrace_event_enabled(current, event))) {
current->ptrace_message = message;
+   current->ptrace_event = event;
ptrace_notify((event << 8) | SIGTRAP);
} else if (event == PTRACE_EVENT_EXEC) {
/* legacy EXEC report via SIGTRAP */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index a51c13c2b1a0..86215fb654d6 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -964,6 +964,7 @@ struct task_struct {
 
/* Ptrace state: */
unsigned long   ptrace_message;
+   int ptrace_event;
kernel_siginfo_t*last_siginfo;
 
struct task_io_accounting   ioac;
diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
index 633a83fe7051..5d2e5aa07a5c 100644
--- a/include/linux/tracehook.h
+++ b/include/linux/tracehook.h
@@ -66,6 +66,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs,
return 0;
 
current->ptrace_message = message;
+   current->ptrace_event = 0;
ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0));
 
/*
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 92c47cd5ad84..74a37e74c7f1 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -904,7 +904,9 @@ static int ptrace_get_syscall(struct task_struct *child,
unsigned long actual_size;
unsigned long write_size;
 
-   if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) {
+   if ((child->ptrace_event == 0 &&
+child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) ||
+   child->ptrace_event == PTRACE_EVENT_SECCOMP) {
int i;
 
info.op = PTRACE_SYSCALL_INFO_ENTRY;
@@ -917,7 +919,8 @@ static int ptrace_get_syscall(struct task_struct *child,
for (i = 0; i < ARRAY_SIZE(args); i++)
info.entry.args[i] = args[i];
actual_size = offsetofend(struct ptrace_syscall_info, entry);
-   } else if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) {
+   } else if (child->ptrace_event == 0 &&
+  child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) {
info.op = PTRACE_SYSCALL_INFO_EXIT;
info.arch = syscall_get_arch(child);
info.exit.rval = syscall_get_error(child, regs);
-- 
2.19.1



[RFC PATCH v3 3/3] ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO

2018-11-24 Thread Elvira Khabirova
Extend PTRACE_GET_SYSCALL_INFO to support PTRACE_EVENT_SECCOMP stops.
The information returned is the same as for syscall-enter-stops.

Signed-off-by: Elvira Khabirova 
Signed-off-by: Dmitry V. Levin 
---
 include/linux/ptrace.h| 1 +
 include/linux/sched.h | 1 +
 include/linux/tracehook.h | 1 +
 kernel/ptrace.c   | 7 +--
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index 6c2ffed907f5..a993d0fde865 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -166,6 +166,7 @@ static inline void ptrace_event(int event, unsigned long 
message)
 {
if (unlikely(ptrace_event_enabled(current, event))) {
current->ptrace_message = message;
+   current->ptrace_event = event;
ptrace_notify((event << 8) | SIGTRAP);
} else if (event == PTRACE_EVENT_EXEC) {
/* legacy EXEC report via SIGTRAP */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index a51c13c2b1a0..86215fb654d6 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -964,6 +964,7 @@ struct task_struct {
 
/* Ptrace state: */
unsigned long   ptrace_message;
+   int ptrace_event;
kernel_siginfo_t*last_siginfo;
 
struct task_io_accounting   ioac;
diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
index 633a83fe7051..5d2e5aa07a5c 100644
--- a/include/linux/tracehook.h
+++ b/include/linux/tracehook.h
@@ -66,6 +66,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs,
return 0;
 
current->ptrace_message = message;
+   current->ptrace_event = 0;
ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0));
 
/*
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 92c47cd5ad84..74a37e74c7f1 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -904,7 +904,9 @@ static int ptrace_get_syscall(struct task_struct *child,
unsigned long actual_size;
unsigned long write_size;
 
-   if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) {
+   if ((child->ptrace_event == 0 &&
+child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_ENTRY) ||
+   child->ptrace_event == PTRACE_EVENT_SECCOMP) {
int i;
 
info.op = PTRACE_SYSCALL_INFO_ENTRY;
@@ -917,7 +919,8 @@ static int ptrace_get_syscall(struct task_struct *child,
for (i = 0; i < ARRAY_SIZE(args); i++)
info.entry.args[i] = args[i];
actual_size = offsetofend(struct ptrace_syscall_info, entry);
-   } else if (child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) {
+   } else if (child->ptrace_event == 0 &&
+  child->ptrace_message == PTRACE_EVENTMSG_SYSCALL_EXIT) {
info.op = PTRACE_SYSCALL_INFO_EXIT;
info.arch = syscall_get_arch(child);
info.exit.rval = syscall_get_error(child, regs);
-- 
2.19.1



[PATCH v3 2/3] ptrace: add PTRACE_GET_SYSCALL_INFO request

2018-11-24 Thread Elvira Khabirova
PTRACE_GET_SYSCALL_INFO lets ptracer obtain details of the syscall
the tracee is blocked in.  The request succeeds when the tracee is in a
syscall-enter-stop or syscall-exit-stop, and fails with -EINVAL otherwise.
A subsequent change may extend PTRACE_GET_SYSCALL_INFO for the case
of PTRACE_EVENT_SECCOMP stop.

There are two reasons for a special syscall-related ptrace request.

Firstly, with the current ptrace API there are cases when ptracer cannot
retrieve necessary information about syscalls.  Some examples include:
* The notorious int-0x80-from-64-bit-task issue.  See [1] for details.
In short, if a 64-bit task performs a syscall through int 0x80, its tracer
has no reliable means to find out that the syscall was, in fact,
a compat syscall, and misidentifies it.
* Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
Common practice is to keep track of the sequence of ptrace-stops in order
not to mix the two syscall-stops up.  But it is not as simple as it looks;
for example, strace had a (just recently fixed) long-standing bug where
attaching strace to a tracee that is performing the execve system call
led to the tracer identifying the following syscall-exit-stop as
syscall-enter-stop, which messed up all the state tracking.
* Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
and process_vm_readv become unavailable when the process dumpable flag
is cleared.  On such architectures as ia64 this results in all syscall
arguments being unavailable.

Secondly, ptracers also have to support a lot of arch-specific code for
obtaining information about the tracee.  For some architectures, this
requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
argument and return value.

ptrace(2) man page:

long ptrace(enum __ptrace_request request, pid_t pid,
void *addr, void *data);
...
PTRACE_GET_SYSCALL_INFO
   Retrieve information about the syscall that caused the stop.
   The information is placed into the buffer pointed by "data"
   argument, which should be a pointer to a buffer of type
   "struct ptrace_syscall_info".
   The "addr" argument contains the size of the buffer pointed to
   by "data" (i.e., sizeof(struct ptrace_syscall_info)).
   The return value contains the number of bytes available
   to be written by the kernel.
   If the size of data to be written by the kernel exceeds the size
   specified by "addr" argument, the output is truncated.
   This operation fails with EINVAL if the tracee is not
   in a syscall-enter-stop, a syscall-exit-stop, or
   a PTRACE_EVENT_SECCOMP stop.

Signed-off-by: Elvira Khabirova 
Signed-off-by: Dmitry V. Levin 
---
 include/uapi/linux/ptrace.h | 24 ++
 kernel/ptrace.c | 50 +
 2 files changed, 74 insertions(+)

diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
index cb138902d042..49b0b1b41943 100644
--- a/include/uapi/linux/ptrace.h
+++ b/include/uapi/linux/ptrace.h
@@ -73,6 +73,30 @@ struct seccomp_metadata {
__u64 flags;/* Output: filter's flags */
 };
 
+#define PTRACE_GET_SYSCALL_INFO0x420f
+#define PTRACE_SYSCALL_INFO_ENTRY  0
+#define PTRACE_SYSCALL_INFO_EXIT   1
+
+struct ptrace_syscall_info {
+   __u8 op;/* PTRACE_SYSCALL_INFO_* */
+   __u8 __pad0[3];
+   __u32 arch;
+   union {
+   struct {
+   __u64 nr;
+   __u64 instruction_pointer;
+   __u64 stack_pointer;
+   __u64 frame_pointer;
+   __u64 args[6];
+   } entry;
+   struct {
+   __s64 rval;
+   __u8 is_error;
+   __u8 __pad1[7];
+   } exit;
+   };
+};
+
 /* Read signals from a shared (process wide) queue */
 #define PTRACE_PEEKSIGINFO_SHARED  (1 << 0)
 
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 80b34dffdfb9..92c47cd5ad84 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -30,6 +30,10 @@
 #include 
 #include 
 
+#ifdef CONFIG_HAVE_ARCH_TRACEHOOK
+#include/* For syscall_get_* */
+#endif
+
 /*
  * Access another process' address space via ptrace.
  * Source/target buffer must be kernel space,
@@ -890,6 +894,46 @@ static int ptrace_regset(struct task_struct *task, int 
req, unsigned int type,
 EXPORT_SYMBOL_GPL(task_user_regset_view);
 #endif
 
+#ifdef CONFIG_HAVE_ARCH_TRACEHOOK
+static int ptrace_get_syscall(struct task_struct *child,
+ unsigned long user_size, void __user *datavp)
+{
+   struct ptrace_syscall_info info;
+   struct pt_regs *regs = task_pt_regs(child);
+   unsigned long args[ARRAY_SIZE(info.entry.args)];
+

[PATCH v3 2/3] ptrace: add PTRACE_GET_SYSCALL_INFO request

2018-11-24 Thread Elvira Khabirova
PTRACE_GET_SYSCALL_INFO lets ptracer obtain details of the syscall
the tracee is blocked in.  The request succeeds when the tracee is in a
syscall-enter-stop or syscall-exit-stop, and fails with -EINVAL otherwise.
A subsequent change may extend PTRACE_GET_SYSCALL_INFO for the case
of PTRACE_EVENT_SECCOMP stop.

There are two reasons for a special syscall-related ptrace request.

Firstly, with the current ptrace API there are cases when ptracer cannot
retrieve necessary information about syscalls.  Some examples include:
* The notorious int-0x80-from-64-bit-task issue.  See [1] for details.
In short, if a 64-bit task performs a syscall through int 0x80, its tracer
has no reliable means to find out that the syscall was, in fact,
a compat syscall, and misidentifies it.
* Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
Common practice is to keep track of the sequence of ptrace-stops in order
not to mix the two syscall-stops up.  But it is not as simple as it looks;
for example, strace had a (just recently fixed) long-standing bug where
attaching strace to a tracee that is performing the execve system call
led to the tracer identifying the following syscall-exit-stop as
syscall-enter-stop, which messed up all the state tracking.
* Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
and process_vm_readv become unavailable when the process dumpable flag
is cleared.  On such architectures as ia64 this results in all syscall
arguments being unavailable.

Secondly, ptracers also have to support a lot of arch-specific code for
obtaining information about the tracee.  For some architectures, this
requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
argument and return value.

ptrace(2) man page:

long ptrace(enum __ptrace_request request, pid_t pid,
void *addr, void *data);
...
PTRACE_GET_SYSCALL_INFO
   Retrieve information about the syscall that caused the stop.
   The information is placed into the buffer pointed by "data"
   argument, which should be a pointer to a buffer of type
   "struct ptrace_syscall_info".
   The "addr" argument contains the size of the buffer pointed to
   by "data" (i.e., sizeof(struct ptrace_syscall_info)).
   The return value contains the number of bytes available
   to be written by the kernel.
   If the size of data to be written by the kernel exceeds the size
   specified by "addr" argument, the output is truncated.
   This operation fails with EINVAL if the tracee is not
   in a syscall-enter-stop, a syscall-exit-stop, or
   a PTRACE_EVENT_SECCOMP stop.

Signed-off-by: Elvira Khabirova 
Signed-off-by: Dmitry V. Levin 
---
 include/uapi/linux/ptrace.h | 24 ++
 kernel/ptrace.c | 50 +
 2 files changed, 74 insertions(+)

diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
index cb138902d042..49b0b1b41943 100644
--- a/include/uapi/linux/ptrace.h
+++ b/include/uapi/linux/ptrace.h
@@ -73,6 +73,30 @@ struct seccomp_metadata {
__u64 flags;/* Output: filter's flags */
 };
 
+#define PTRACE_GET_SYSCALL_INFO0x420f
+#define PTRACE_SYSCALL_INFO_ENTRY  0
+#define PTRACE_SYSCALL_INFO_EXIT   1
+
+struct ptrace_syscall_info {
+   __u8 op;/* PTRACE_SYSCALL_INFO_* */
+   __u8 __pad0[3];
+   __u32 arch;
+   union {
+   struct {
+   __u64 nr;
+   __u64 instruction_pointer;
+   __u64 stack_pointer;
+   __u64 frame_pointer;
+   __u64 args[6];
+   } entry;
+   struct {
+   __s64 rval;
+   __u8 is_error;
+   __u8 __pad1[7];
+   } exit;
+   };
+};
+
 /* Read signals from a shared (process wide) queue */
 #define PTRACE_PEEKSIGINFO_SHARED  (1 << 0)
 
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 80b34dffdfb9..92c47cd5ad84 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -30,6 +30,10 @@
 #include 
 #include 
 
+#ifdef CONFIG_HAVE_ARCH_TRACEHOOK
+#include/* For syscall_get_* */
+#endif
+
 /*
  * Access another process' address space via ptrace.
  * Source/target buffer must be kernel space,
@@ -890,6 +894,46 @@ static int ptrace_regset(struct task_struct *task, int 
req, unsigned int type,
 EXPORT_SYMBOL_GPL(task_user_regset_view);
 #endif
 
+#ifdef CONFIG_HAVE_ARCH_TRACEHOOK
+static int ptrace_get_syscall(struct task_struct *child,
+ unsigned long user_size, void __user *datavp)
+{
+   struct ptrace_syscall_info info;
+   struct pt_regs *regs = task_pt_regs(child);
+   unsigned long args[ARRAY_SIZE(info.entry.args)];
+

[PATCH v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message

2018-11-24 Thread Elvira Khabirova
Define two constants, PTRACE_EVENTMSG_SYSCALL_ENTRY and
PTRACE_EVENTMSG_SYSCALL_EXIT, and place them in ptrace_message
for the duration of syscall-stops.
This way ptracers can distinguish syscall-enter-stops
from syscall-exit-stops using PTRACE_GETEVENTMSG request.

Signed-off-by: Elvira Khabirova 
Signed-off-by: Dmitry V. Levin 
---
 include/linux/tracehook.h   |  9 ++---
 include/uapi/linux/ptrace.h | 10 ++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
index 40b0b4c1bf7b..633a83fe7051 100644
--- a/include/linux/tracehook.h
+++ b/include/linux/tracehook.h
@@ -57,13 +57,15 @@ struct linux_binprm;
 /*
  * ptrace report for syscall entry and exit looks identical.
  */
-static inline int ptrace_report_syscall(struct pt_regs *regs)
+static inline int ptrace_report_syscall(struct pt_regs *regs,
+   unsigned long message)
 {
int ptrace = current->ptrace;
 
if (!(ptrace & PT_PTRACED))
return 0;
 
+   current->ptrace_message = message;
ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0));
 
/*
@@ -76,6 +78,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs)
current->exit_code = 0;
}
 
+   current->ptrace_message = 0;
return fatal_signal_pending(current);
 }
 
@@ -101,7 +104,7 @@ static inline int ptrace_report_syscall(struct pt_regs 
*regs)
 static inline __must_check int tracehook_report_syscall_entry(
struct pt_regs *regs)
 {
-   return ptrace_report_syscall(regs);
+   return ptrace_report_syscall(regs, PTRACE_EVENTMSG_SYSCALL_ENTRY);
 }
 
 /**
@@ -126,7 +129,7 @@ static inline void tracehook_report_syscall_exit(struct 
pt_regs *regs, int step)
if (step)
user_single_step_report(regs);
else
-   ptrace_report_syscall(regs);
+   ptrace_report_syscall(regs, PTRACE_EVENTMSG_SYSCALL_EXIT);
 }
 
 /**
diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
index d5a1b8a492b9..cb138902d042 100644
--- a/include/uapi/linux/ptrace.h
+++ b/include/uapi/linux/ptrace.h
@@ -104,6 +104,16 @@ struct seccomp_metadata {
 #define PTRACE_O_MASK  (\
0x00ff | PTRACE_O_EXITKILL | PTRACE_O_SUSPEND_SECCOMP)
 
+/*
+ * These values are stored in task->ptrace_message by 
tracehook_report_syscall_*
+ * to describe current syscall-stop.
+ *
+ * Values for these constants are chosen so that they do not appear
+ * in task->ptrace_message by other means.
+ */
+#define PTRACE_EVENTMSG_SYSCALL_ENTRY  0x8000U
+#define PTRACE_EVENTMSG_SYSCALL_EXIT   0x9000U
+
 #include 
 
 
-- 
2.19.1



[PATCH v3 1/3] ptrace: pass type of a syscall-stop in ptrace_message

2018-11-24 Thread Elvira Khabirova
Define two constants, PTRACE_EVENTMSG_SYSCALL_ENTRY and
PTRACE_EVENTMSG_SYSCALL_EXIT, and place them in ptrace_message
for the duration of syscall-stops.
This way ptracers can distinguish syscall-enter-stops
from syscall-exit-stops using PTRACE_GETEVENTMSG request.

Signed-off-by: Elvira Khabirova 
Signed-off-by: Dmitry V. Levin 
---
 include/linux/tracehook.h   |  9 ++---
 include/uapi/linux/ptrace.h | 10 ++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
index 40b0b4c1bf7b..633a83fe7051 100644
--- a/include/linux/tracehook.h
+++ b/include/linux/tracehook.h
@@ -57,13 +57,15 @@ struct linux_binprm;
 /*
  * ptrace report for syscall entry and exit looks identical.
  */
-static inline int ptrace_report_syscall(struct pt_regs *regs)
+static inline int ptrace_report_syscall(struct pt_regs *regs,
+   unsigned long message)
 {
int ptrace = current->ptrace;
 
if (!(ptrace & PT_PTRACED))
return 0;
 
+   current->ptrace_message = message;
ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0));
 
/*
@@ -76,6 +78,7 @@ static inline int ptrace_report_syscall(struct pt_regs *regs)
current->exit_code = 0;
}
 
+   current->ptrace_message = 0;
return fatal_signal_pending(current);
 }
 
@@ -101,7 +104,7 @@ static inline int ptrace_report_syscall(struct pt_regs 
*regs)
 static inline __must_check int tracehook_report_syscall_entry(
struct pt_regs *regs)
 {
-   return ptrace_report_syscall(regs);
+   return ptrace_report_syscall(regs, PTRACE_EVENTMSG_SYSCALL_ENTRY);
 }
 
 /**
@@ -126,7 +129,7 @@ static inline void tracehook_report_syscall_exit(struct 
pt_regs *regs, int step)
if (step)
user_single_step_report(regs);
else
-   ptrace_report_syscall(regs);
+   ptrace_report_syscall(regs, PTRACE_EVENTMSG_SYSCALL_EXIT);
 }
 
 /**
diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
index d5a1b8a492b9..cb138902d042 100644
--- a/include/uapi/linux/ptrace.h
+++ b/include/uapi/linux/ptrace.h
@@ -104,6 +104,16 @@ struct seccomp_metadata {
 #define PTRACE_O_MASK  (\
0x00ff | PTRACE_O_EXITKILL | PTRACE_O_SUSPEND_SECCOMP)
 
+/*
+ * These values are stored in task->ptrace_message by 
tracehook_report_syscall_*
+ * to describe current syscall-stop.
+ *
+ * Values for these constants are chosen so that they do not appear
+ * in task->ptrace_message by other means.
+ */
+#define PTRACE_EVENTMSG_SYSCALL_ENTRY  0x8000U
+#define PTRACE_EVENTMSG_SYSCALL_EXIT   0x9000U
+
 #include 
 
 
-- 
2.19.1



[PATCH v3 0/3] ptrace: add PTRACE_GET_SYSCALL_INFO request

2018-11-24 Thread Elvira Khabirova
PTRACE_GET_SYSCALL_INFO lets ptracer obtain details of the syscall
the tracee is blocked in.  The request succeeds when the tracee is in a
syscall-enter-stop, syscall-exit-stop or PTRACE_EVENT_SECCOMP stop,
and fails with -EINVAL otherwise.

There are two reasons for a special syscall-related ptrace request.

Firstly, with the current ptrace API there are cases when ptracer cannot
retrieve necessary information about syscalls.  Some examples include:
* The notorious int-0x80-from-64-bit-task issue.  See [1] for details.
In short, if a 64-bit task performs a syscall through int 0x80, its tracer
has no reliable means to find out that the syscall was, in fact,
a compat syscall, and misidentifies it.
* Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
Common practice is to keep track of the sequence of ptrace-stops in order
not to mix the two syscall-stops up.  But it is not as simple as it looks;
for example, strace had a (just recently fixed) long-standing bug where
attaching strace to a tracee that is performing the execve system call
led to the tracer identifying the following syscall-exit-stop as
syscall-enter-stop, which messed up all the state tracking.
* Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
and process_vm_readv become unavailable when the process dumpable flag
is cleared.  On such architectures as ia64 this results in all syscall
arguments being unavailable.

Secondly, ptracers also have to support a lot of arch-specific code for
obtaining information about the tracee.  For some architectures, this
requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
argument and return value.

PTRACE_GET_SYSCALL_INFO returns the following structure:

struct ptrace_syscall_info {
__u8 op;/* PTRACE_SYSCALL_INFO_* */
__u8 __pad0[3];
__u32 arch;
union {
struct {
__u64 nr;
__u64 instruction_pointer;
__u64 stack_pointer;
__u64 frame_pointer;
__u64 args[6];
} entry;
struct {
__s64 rval;
__u8 is_error;
__u8 __pad1[7];
} exit;
};
};

The structure was chosen according to [2], except for the following
changes:
* arch is returned unconditionally to aid with tracing system calls such as
execve();
* the type of nr field was changed from int to __u64 because syscall
numbers are, as a practical matter, 64 bits;
* stack_pointer and frame_pointer fields were added along with
instruction_pointer field since they are readily available and can save
the tracer from extra PTRACE_GETREGSET calls;
* a boolean is_error field was added along with rval field, this way
the tracer can more reliably distinguish a return value
from an error value.

This changeset should be applied on top of [3] and [4].

[1] 
https://lore.kernel.org/lkml/ca+55afzcsvmddj9lh_gdbz1ozhyem6zrgpbdajnywm2lf_e...@mail.gmail.com/
[2] 
https://lore.kernel.org/lkml/caobl_7gm0n80n7j_dfw_eqyflyzq+sf4y2avsccv88tb3aw...@mail.gmail.com/
[3] https://lore.kernel.org/lkml/20181119210139.ga8...@altlinux.org/
[4] https://lore.kernel.org/lkml/20181120001128.ga11...@altlinux.org/

v3: Split into three changes.
Change struct ptrace_syscall_info.
Support PTRACE_EVENT_SECCOMP by adding ptrace_event to task_struct.
Add proper defines for ptrace_syscall_info.op values.
Rename PT_SYSCALL_IS_ENTERING and PT_SYSCALL_IS_EXITING to
PTRACE_EVENTMSG_SYSCALL_ENTRY and PTRACE_EVENTMSG_SYSCALL_EXIT and move
them to uapi.

Elvira Khabirova (3):
  ptrace: pass type of a syscall-stop in ptrace_message
  ptrace: add PTRACE_GET_SYSCALL_INFO request
  ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO

 include/linux/ptrace.h  |  1 +
 include/linux/sched.h   |  1 +
 include/linux/tracehook.h   | 10 ---
 include/uapi/linux/ptrace.h | 34 
 kernel/ptrace.c | 53 +
 5 files changed, 96 insertions(+), 3 deletions(-)

-- 
2.19.1



[PATCH v3 0/3] ptrace: add PTRACE_GET_SYSCALL_INFO request

2018-11-24 Thread Elvira Khabirova
PTRACE_GET_SYSCALL_INFO lets ptracer obtain details of the syscall
the tracee is blocked in.  The request succeeds when the tracee is in a
syscall-enter-stop, syscall-exit-stop or PTRACE_EVENT_SECCOMP stop,
and fails with -EINVAL otherwise.

There are two reasons for a special syscall-related ptrace request.

Firstly, with the current ptrace API there are cases when ptracer cannot
retrieve necessary information about syscalls.  Some examples include:
* The notorious int-0x80-from-64-bit-task issue.  See [1] for details.
In short, if a 64-bit task performs a syscall through int 0x80, its tracer
has no reliable means to find out that the syscall was, in fact,
a compat syscall, and misidentifies it.
* Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
Common practice is to keep track of the sequence of ptrace-stops in order
not to mix the two syscall-stops up.  But it is not as simple as it looks;
for example, strace had a (just recently fixed) long-standing bug where
attaching strace to a tracee that is performing the execve system call
led to the tracer identifying the following syscall-exit-stop as
syscall-enter-stop, which messed up all the state tracking.
* Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
and process_vm_readv become unavailable when the process dumpable flag
is cleared.  On such architectures as ia64 this results in all syscall
arguments being unavailable.

Secondly, ptracers also have to support a lot of arch-specific code for
obtaining information about the tracee.  For some architectures, this
requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
argument and return value.

PTRACE_GET_SYSCALL_INFO returns the following structure:

struct ptrace_syscall_info {
__u8 op;/* PTRACE_SYSCALL_INFO_* */
__u8 __pad0[3];
__u32 arch;
union {
struct {
__u64 nr;
__u64 instruction_pointer;
__u64 stack_pointer;
__u64 frame_pointer;
__u64 args[6];
} entry;
struct {
__s64 rval;
__u8 is_error;
__u8 __pad1[7];
} exit;
};
};

The structure was chosen according to [2], except for the following
changes:
* arch is returned unconditionally to aid with tracing system calls such as
execve();
* the type of nr field was changed from int to __u64 because syscall
numbers are, as a practical matter, 64 bits;
* stack_pointer and frame_pointer fields were added along with
instruction_pointer field since they are readily available and can save
the tracer from extra PTRACE_GETREGSET calls;
* a boolean is_error field was added along with rval field, this way
the tracer can more reliably distinguish a return value
from an error value.

This changeset should be applied on top of [3] and [4].

[1] 
https://lore.kernel.org/lkml/ca+55afzcsvmddj9lh_gdbz1ozhyem6zrgpbdajnywm2lf_e...@mail.gmail.com/
[2] 
https://lore.kernel.org/lkml/caobl_7gm0n80n7j_dfw_eqyflyzq+sf4y2avsccv88tb3aw...@mail.gmail.com/
[3] https://lore.kernel.org/lkml/20181119210139.ga8...@altlinux.org/
[4] https://lore.kernel.org/lkml/20181120001128.ga11...@altlinux.org/

v3: Split into three changes.
Change struct ptrace_syscall_info.
Support PTRACE_EVENT_SECCOMP by adding ptrace_event to task_struct.
Add proper defines for ptrace_syscall_info.op values.
Rename PT_SYSCALL_IS_ENTERING and PT_SYSCALL_IS_EXITING to
PTRACE_EVENTMSG_SYSCALL_ENTRY and PTRACE_EVENTMSG_SYSCALL_EXIT and move
them to uapi.

Elvira Khabirova (3):
  ptrace: pass type of a syscall-stop in ptrace_message
  ptrace: add PTRACE_GET_SYSCALL_INFO request
  ptrace: add PTRACE_EVENT_SECCOMP support to PTRACE_GET_SYSCALL_INFO

 include/linux/ptrace.h  |  1 +
 include/linux/sched.h   |  1 +
 include/linux/tracehook.h   | 10 ---
 include/uapi/linux/ptrace.h | 34 
 kernel/ptrace.c | 53 +
 5 files changed, 96 insertions(+), 3 deletions(-)

-- 
2.19.1



[RFC PATCH v2] ptrace: add PTRACE_GET_SYSCALL_INFO request

2018-11-21 Thread Elvira Khabirova
PTRACE_GET_SYSCALL_INFO lets ptracer obtain details of the syscall
the tracee is blocked in.  The request returns meaningful data only
when the tracee is in a syscall-enter-stop or a syscall-exit-stop.

There are two reasons for a special syscall-related ptrace request.

Firstly, with the current ptrace API there are cases when ptracer cannot
retrieve necessary information about syscalls.  Some examples include:
* The notorious int-0x80-from-64-bit-task issue.  See [1] for details.
In short, if a 64-bit task performs a syscall through int 0x80, its tracer
has no reliable means to find out that the syscall was, in fact,
a compat syscall, and misidentifies it.
* Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
Common practice is to keep track of the sequence of ptrace-stops in order
not to mix the two syscall-stops up.  But it is not as simple as it looks;
for example, strace had a (just recently fixed) long-standing bug where
attaching strace to a tracee that is performing the execve system call
led to the tracer identifying the following syscall-exit-stop as
syscall-enter-stop, which messed up all the state tracking.
* Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
and process_vm_readv become unavailable when the process dumpable flag
is cleared.  On such architectures as ia64 this results in all syscall
arguments being unavailable.

Secondly, ptracers also have to support a lot of arch-specific code for
obtaining information about the tracee.  For some architectures, this
requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
argument and return value.

PTRACE_GET_SYSCALL_INFO returns the following structure:

struct ptrace_syscall_info {
__u8 op; /* 0 for entry, 1 for exit */
__u8 __pad0[7];
union {
struct {
__s32 nr;
__u32 arch;
__u64 instruction_pointer;
__u64 args[6];
} entry_info;
struct {
__s64 rval;
__u8 is_error;
__u8 __pad1[7];
} exit_info;
};
};

The structure was chosen according to [2], except for one change:
a boolean is_error field is added along with rval.  This way the tracer
can more reliably distinguish a return value from an error value.

This patch should be applied on top of [3] and [4].

[1] 
https://lore.kernel.org/lkml/ca+55afzcsvmddj9lh_gdbz1ozhyem6zrgpbdajnywm2lf_e...@mail.gmail.com/
[2] 
https://lore.kernel.org/lkml/caobl_7gm0n80n7j_dfw_eqyflyzq+sf4y2avsccv88tb3aw...@mail.gmail.com/
[3] https://lore.kernel.org/lkml/20181119210139.ga8...@altlinux.org/
[4] https://lore.kernel.org/lkml/20181120001128.ga11...@altlinux.org/

Co-authored-by: Dmitry V. Levin 
Signed-off-by: Elvira Khabirova 
Signed-off-by: Dmitry V. Levin 
---
Changes since v1:
 * Do not use task->ptrace.
 * Replace entry_info.is_compat with entry_info.arch, use syscall_get_arch().
 * Use addr argument of sys_ptrace to get expected size of the struct;
   return full size of the struct.

 include/linux/ptrace.h  |  8 ++
 include/linux/tracehook.h   |  9 --
 include/uapi/linux/ptrace.h | 20 +
 kernel/ptrace.c | 56 +
 4 files changed, 90 insertions(+), 3 deletions(-)

diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index 6c2ffed907f5..909930c893d0 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -46,6 +46,14 @@ extern int ptrace_access_vm(struct task_struct *tsk, 
unsigned long addr,
 #define PT_BLOCKSTEP_BIT   30
 #define PT_BLOCKSTEP   (1<ptrace_message
+ * for later use by PTRACE_GET_SYSCALL_INFO.
+ */
+#define PT_SYSCALL_IS_ENTERING  0x8000U
+#define PT_SYSCALL_IS_EXITING   0x9000U
+
 extern long arch_ptrace(struct task_struct *child, long request,
unsigned long addr, unsigned long data);
 extern int ptrace_readdata(struct task_struct *tsk, unsigned long src, char 
__user *dst, int len);
diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
index 40b0b4c1bf7b..24d0e2215ed2 100644
--- a/include/linux/tracehook.h
+++ b/include/linux/tracehook.h
@@ -57,13 +57,15 @@ struct linux_binprm;
 /*
  * ptrace report for syscall entry and exit looks identical.
  */
-static inline int ptrace_report_syscall(struct pt_regs *regs)
+static inline int ptrace_report_syscall(struct pt_regs *regs,
+   unsigned long message)
 {
int ptrace = current->ptrace;
 
if (!(ptrace & PT_PTRACED))
return 0;
 
+   current->ptrace_message = message;
ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0));
 
/*
@@ -76,6 +78,7 @@ static inline int ptrace_repor

[RFC PATCH v2] ptrace: add PTRACE_GET_SYSCALL_INFO request

2018-11-21 Thread Elvira Khabirova
PTRACE_GET_SYSCALL_INFO lets ptracer obtain details of the syscall
the tracee is blocked in.  The request returns meaningful data only
when the tracee is in a syscall-enter-stop or a syscall-exit-stop.

There are two reasons for a special syscall-related ptrace request.

Firstly, with the current ptrace API there are cases when ptracer cannot
retrieve necessary information about syscalls.  Some examples include:
* The notorious int-0x80-from-64-bit-task issue.  See [1] for details.
In short, if a 64-bit task performs a syscall through int 0x80, its tracer
has no reliable means to find out that the syscall was, in fact,
a compat syscall, and misidentifies it.
* Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
Common practice is to keep track of the sequence of ptrace-stops in order
not to mix the two syscall-stops up.  But it is not as simple as it looks;
for example, strace had a (just recently fixed) long-standing bug where
attaching strace to a tracee that is performing the execve system call
led to the tracer identifying the following syscall-exit-stop as
syscall-enter-stop, which messed up all the state tracking.
* Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
and process_vm_readv become unavailable when the process dumpable flag
is cleared.  On such architectures as ia64 this results in all syscall
arguments being unavailable.

Secondly, ptracers also have to support a lot of arch-specific code for
obtaining information about the tracee.  For some architectures, this
requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
argument and return value.

PTRACE_GET_SYSCALL_INFO returns the following structure:

struct ptrace_syscall_info {
__u8 op; /* 0 for entry, 1 for exit */
__u8 __pad0[7];
union {
struct {
__s32 nr;
__u32 arch;
__u64 instruction_pointer;
__u64 args[6];
} entry_info;
struct {
__s64 rval;
__u8 is_error;
__u8 __pad1[7];
} exit_info;
};
};

The structure was chosen according to [2], except for one change:
a boolean is_error field is added along with rval.  This way the tracer
can more reliably distinguish a return value from an error value.

This patch should be applied on top of [3] and [4].

[1] 
https://lore.kernel.org/lkml/ca+55afzcsvmddj9lh_gdbz1ozhyem6zrgpbdajnywm2lf_e...@mail.gmail.com/
[2] 
https://lore.kernel.org/lkml/caobl_7gm0n80n7j_dfw_eqyflyzq+sf4y2avsccv88tb3aw...@mail.gmail.com/
[3] https://lore.kernel.org/lkml/20181119210139.ga8...@altlinux.org/
[4] https://lore.kernel.org/lkml/20181120001128.ga11...@altlinux.org/

Co-authored-by: Dmitry V. Levin 
Signed-off-by: Elvira Khabirova 
Signed-off-by: Dmitry V. Levin 
---
Changes since v1:
 * Do not use task->ptrace.
 * Replace entry_info.is_compat with entry_info.arch, use syscall_get_arch().
 * Use addr argument of sys_ptrace to get expected size of the struct;
   return full size of the struct.

 include/linux/ptrace.h  |  8 ++
 include/linux/tracehook.h   |  9 --
 include/uapi/linux/ptrace.h | 20 +
 kernel/ptrace.c | 56 +
 4 files changed, 90 insertions(+), 3 deletions(-)

diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index 6c2ffed907f5..909930c893d0 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -46,6 +46,14 @@ extern int ptrace_access_vm(struct task_struct *tsk, 
unsigned long addr,
 #define PT_BLOCKSTEP_BIT   30
 #define PT_BLOCKSTEP   (1<ptrace_message
+ * for later use by PTRACE_GET_SYSCALL_INFO.
+ */
+#define PT_SYSCALL_IS_ENTERING  0x8000U
+#define PT_SYSCALL_IS_EXITING   0x9000U
+
 extern long arch_ptrace(struct task_struct *child, long request,
unsigned long addr, unsigned long data);
 extern int ptrace_readdata(struct task_struct *tsk, unsigned long src, char 
__user *dst, int len);
diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
index 40b0b4c1bf7b..24d0e2215ed2 100644
--- a/include/linux/tracehook.h
+++ b/include/linux/tracehook.h
@@ -57,13 +57,15 @@ struct linux_binprm;
 /*
  * ptrace report for syscall entry and exit looks identical.
  */
-static inline int ptrace_report_syscall(struct pt_regs *regs)
+static inline int ptrace_report_syscall(struct pt_regs *regs,
+   unsigned long message)
 {
int ptrace = current->ptrace;
 
if (!(ptrace & PT_PTRACED))
return 0;
 
+   current->ptrace_message = message;
ptrace_notify(SIGTRAP | ((ptrace & PT_TRACESYSGOOD) ? 0x80 : 0));
 
/*
@@ -76,6 +78,7 @@ static inline int ptrace_repor

[PATCH v2] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call

2018-11-16 Thread Elvira Khabirova
Arch code should use tracehook_*() helpers, as documented
in include/linux/tracehook.h.

Fixes: 5521eb4bca2d ("powerpc/ptrace: Add support for PTRACE_SYSEMU")
Signed-off-by: Elvira Khabirova 
---
 arch/powerpc/kernel/ptrace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index afb819f4ca68..d79d907f9acc 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -3266,7 +3266,7 @@ long do_syscall_trace_enter(struct pt_regs *regs)
user_exit();
 
if (test_thread_flag(TIF_SYSCALL_EMU)) {
-   ptrace_report_syscall(regs);
+   (void) tracehook_report_syscall_entry(regs);
/*
 * Returning -1 will skip the syscall execution. We want to
 * avoid clobbering any register also, thus, not 'gotoing'
-- 
2.19.1



[tip:x86/urgent] x86/ptrace: Fix documentation for tracehook_report_syscall_entry()

2018-11-11 Thread tip-bot for Elvira Khabirova
Commit-ID:  d19f9130b814d33c03118493c17454f7d90075d1
Gitweb: https://git.kernel.org/tip/d19f9130b814d33c03118493c17454f7d90075d1
Author: Elvira Khabirova 
AuthorDate: Sat, 10 Nov 2018 04:22:09 +0100
Committer:  Ingo Molnar 
CommitDate: Mon, 12 Nov 2018 04:53:27 +0100

x86/ptrace: Fix documentation for tracehook_report_syscall_entry()

tracehook_report_syscall_entry() is called not only
if %TIF_SYSCALL_TRACE is set, but also if %TIF_SYSCALL_EMU is set,
as appears from x86's entry code.

Signed-off-by: Elvira Khabirova 
Cc: Borislav Petkov 
Cc: H. Peter Anvin 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: l...@altlinux.org
Cc: o...@redhat.com
Cc: rost...@goodmis.org
Link: http://lkml.kernel.org/r/20181110042209.26333972@akathisia
Signed-off-by: Ingo Molnar 
---
 include/linux/tracehook.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
index 40b0b4c1bf7b..df20f8bdbfa3 100644
--- a/include/linux/tracehook.h
+++ b/include/linux/tracehook.h
@@ -83,8 +83,8 @@ static inline int ptrace_report_syscall(struct pt_regs *regs)
  * tracehook_report_syscall_entry - task is about to attempt a system call
  * @regs:  user register state of current task
  *
- * This will be called if %TIF_SYSCALL_TRACE has been set, when the
- * current task has just entered the kernel for a system call.
+ * This will be called if %TIF_SYSCALL_TRACE or %TIF_SYSCALL_EMU have been set,
+ * when the current task has just entered the kernel for a system call.
  * Full user register state is available here.  Changing the values
  * in @regs can affect the system call number and arguments to be tried.
  * It is safe to block here, preventing the system call from beginning.


[tip:x86/urgent] x86/ptrace: Fix documentation for tracehook_report_syscall_entry()

2018-11-11 Thread tip-bot for Elvira Khabirova
Commit-ID:  d19f9130b814d33c03118493c17454f7d90075d1
Gitweb: https://git.kernel.org/tip/d19f9130b814d33c03118493c17454f7d90075d1
Author: Elvira Khabirova 
AuthorDate: Sat, 10 Nov 2018 04:22:09 +0100
Committer:  Ingo Molnar 
CommitDate: Mon, 12 Nov 2018 04:53:27 +0100

x86/ptrace: Fix documentation for tracehook_report_syscall_entry()

tracehook_report_syscall_entry() is called not only
if %TIF_SYSCALL_TRACE is set, but also if %TIF_SYSCALL_EMU is set,
as appears from x86's entry code.

Signed-off-by: Elvira Khabirova 
Cc: Borislav Petkov 
Cc: H. Peter Anvin 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: l...@altlinux.org
Cc: o...@redhat.com
Cc: rost...@goodmis.org
Link: http://lkml.kernel.org/r/20181110042209.26333972@akathisia
Signed-off-by: Ingo Molnar 
---
 include/linux/tracehook.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
index 40b0b4c1bf7b..df20f8bdbfa3 100644
--- a/include/linux/tracehook.h
+++ b/include/linux/tracehook.h
@@ -83,8 +83,8 @@ static inline int ptrace_report_syscall(struct pt_regs *regs)
  * tracehook_report_syscall_entry - task is about to attempt a system call
  * @regs:  user register state of current task
  *
- * This will be called if %TIF_SYSCALL_TRACE has been set, when the
- * current task has just entered the kernel for a system call.
+ * This will be called if %TIF_SYSCALL_TRACE or %TIF_SYSCALL_EMU have been set,
+ * when the current task has just entered the kernel for a system call.
  * Full user register state is available here.  Changing the values
  * in @regs can affect the system call number and arguments to be tried.
  * It is safe to block here, preventing the system call from beginning.


[PATCH] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call

2018-11-11 Thread Elvira Khabirova
Arch code should use tracehook_*() helpers, as documented
in include/linux/tracehook.h.

Signed-off-by: Elvira Khabirova 
---
 arch/powerpc/kernel/ptrace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index afb819f4ca68..f73f8ea402e9 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -3266,7 +3266,7 @@ long do_syscall_trace_enter(struct pt_regs *regs)
user_exit();
 
if (test_thread_flag(TIF_SYSCALL_EMU)) {
-   ptrace_report_syscall(regs);
+   tracehook_report_syscall_entry(regs);
/*
 * Returning -1 will skip the syscall execution. We want to
 * avoid clobbering any register also, thus, not 'gotoing'
-- 
2.19.1



[PATCH] tracehook: fix documentation for tracehook_report_syscall_entry()

2018-11-09 Thread Elvira Khabirova
tracehook_report_syscall_entry() is called not only
if %TIF_SYSCALL_TRACE is set, but also if %TIF_SYSCALL_EMU is set,
as appears from x86's entry code.

Signed-off-by: Elvira Khabirova 
---
 include/linux/tracehook.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
index 40b0b4c1bf7b..df20f8bdbfa3 100644
--- a/include/linux/tracehook.h
+++ b/include/linux/tracehook.h
@@ -83,8 +83,8 @@ static inline int ptrace_report_syscall(struct pt_regs *regs)
  * tracehook_report_syscall_entry - task is about to attempt a system call
  * @regs:  user register state of current task
  *
- * This will be called if %TIF_SYSCALL_TRACE has been set, when the
- * current task has just entered the kernel for a system call.
+ * This will be called if %TIF_SYSCALL_TRACE or %TIF_SYSCALL_EMU have been set,
+ * when the current task has just entered the kernel for a system call.
  * Full user register state is available here.  Changing the values
  * in @regs can affect the system call number and arguments to be tried.
  * It is safe to block here, preventing the system call from beginning.
-- 
2.19.1



[PATCH] tracehook: fix documentation for tracehook_report_syscall_entry()

2018-11-09 Thread Elvira Khabirova
tracehook_report_syscall_entry() is called not only
if %TIF_SYSCALL_TRACE is set, but also if %TIF_SYSCALL_EMU is set,
as appears from x86's entry code.

Signed-off-by: Elvira Khabirova 
---
 include/linux/tracehook.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
index 40b0b4c1bf7b..df20f8bdbfa3 100644
--- a/include/linux/tracehook.h
+++ b/include/linux/tracehook.h
@@ -83,8 +83,8 @@ static inline int ptrace_report_syscall(struct pt_regs *regs)
  * tracehook_report_syscall_entry - task is about to attempt a system call
  * @regs:  user register state of current task
  *
- * This will be called if %TIF_SYSCALL_TRACE has been set, when the
- * current task has just entered the kernel for a system call.
+ * This will be called if %TIF_SYSCALL_TRACE or %TIF_SYSCALL_EMU have been set,
+ * when the current task has just entered the kernel for a system call.
  * Full user register state is available here.  Changing the values
  * in @regs can affect the system call number and arguments to be tried.
  * It is safe to block here, preventing the system call from beginning.
-- 
2.19.1



Re: [RFC PATCH] ptrace: add PTRACE_GET_SYSCALL_INFO request

2018-11-07 Thread Elvira Khabirova
On Wed, 7 Nov 2018 17:44:44 +0100
Oleg Nesterov  wrote:

> To me PT_IN_SYSCALL_STOP makes no real sense, but I won't argue.
> 
> At least I'd ask to not abuse task->ptrace. ptrace_report_syscall() can clear
> ->ptrace_message on exit if we really want PTRACE_GET_SYSCALL_INFO to fail 
> after  
> that.

I really would not like to rely on ->ptrace_message remaining empty;
this looks too fragile.


Re: [RFC PATCH] ptrace: add PTRACE_GET_SYSCALL_INFO request

2018-11-07 Thread Elvira Khabirova
On Wed, 7 Nov 2018 17:44:44 +0100
Oleg Nesterov  wrote:

> To me PT_IN_SYSCALL_STOP makes no real sense, but I won't argue.
> 
> At least I'd ask to not abuse task->ptrace. ptrace_report_syscall() can clear
> ->ptrace_message on exit if we really want PTRACE_GET_SYSCALL_INFO to fail 
> after  
> that.

I really would not like to rely on ->ptrace_message remaining empty;
this looks too fragile.


[RFC PATCH] ptrace: add PTRACE_GET_SYSCALL_INFO request

2018-11-06 Thread Elvira Khabirova
PTRACE_GET_SYSCALL_INFO lets ptracer obtain details of the syscall
the tracee is blocked in. The request returns meaningful data only
when the tracee is in a syscall-enter-stop or a syscall-exit-stop.

There are two reasons for a special syscall-related ptrace request.

Firstly, with the current ptrace API there are cases when ptracer cannot
retrieve necessary information about syscalls. Some examples include:
* The notorious int-0x80-from-64-bit-task issue. See [1] for details.
In short, if a 64-bit task performs a syscall through int 0x80, its tracer
has no reliable means to find out that the syscall was, in fact,
a compat syscall, and misidentifies it.
* Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
Common practice is to keep track of the sequence of ptrace-stops in order
not to mix the two syscall-stops up. But it is not as simple as it looks;
for example, strace had a (just recently fixed) long-standing bug where
attaching strace to a tracee that is performing the execve system call
led to the tracer identifying the following syscall-exit-stop as
syscall-enter-stop, which messed up all the state tracking.
* Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
and process_vm_readv become unavailable when the process dumpable flag
is cleared. On ia64 this results in all syscall arguments being unavailable.

Secondly, ptracers also have to support a lot of arch-specific code for
obtaining information about the tracee. For some architectures, this
requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
argument and return value.

PTRACE_GET_SYSCALL_INFO returns the following structure:

struct ptrace_syscall_info {
__u8 op; /* 0 for entry, 1 for exit */
__u8 __pad0[7];
union {
struct {
__u64 nr;
__u64 ip;
__u64 args[6];
__u8 is_compat;
__u8 __pad1[7];
} entry_info;
struct {
__s64 rval;
__u8 is_error;
__u8 __pad2[7];
} exit_info;
};
};

The structure was chosen according to [2], except for two changes.
First: instead of an arch field with a value of AUDIT_ARCH_*, a boolean
is_compat value is returned, because a) not all arches have an AUDIT_ARCH_*
defined for them, b) the tracer already knows what *arch* it is running on,
but it does not know whether the tracee/syscall is in compat mode or not.
Second: a boolean is_error value is added to rval. This way the tracer can
more reliably distinguish a return value from an error value.

[1] 
https://lkml.kernel.org/r/ca+55afzcsvmddj9lh_gdbz1ozhyem6zrgpbdajnywm2lf_e...@mail.gmail.com
[2] 
http://lkml.kernel.org/r/caobl_7gm0n80n7j_dfw_eqyflyzq+sf4y2avsccv88tb3aw...@mail.gmail.com

Signed-off-by: Elvira Khabirova 
---
 arch/alpha/kernel/ptrace.c  |  2 +-
 arch/arc/kernel/ptrace.c|  2 +-
 arch/arm/kernel/ptrace.c|  2 +-
 arch/arm64/kernel/ptrace.c  |  2 +-
 arch/c6x/kernel/ptrace.c|  2 +-
 arch/h8300/kernel/ptrace.c  |  2 +-
 arch/hexagon/kernel/traps.c |  2 +-
 arch/ia64/kernel/ptrace.c   |  2 +-
 arch/m68k/kernel/ptrace.c   |  3 ++-
 arch/microblaze/kernel/ptrace.c |  2 +-
 arch/mips/kernel/ptrace.c   |  2 +-
 arch/nds32/kernel/ptrace.c  |  2 +-
 arch/nios2/kernel/ptrace.c  |  3 ++-
 arch/openrisc/kernel/ptrace.c   |  2 +-
 arch/parisc/kernel/ptrace.c |  2 +-
 arch/powerpc/kernel/ptrace.c|  2 +-
 arch/riscv/kernel/ptrace.c  |  2 +-
 arch/s390/kernel/ptrace.c   |  2 +-
 arch/sh/kernel/ptrace_32.c  |  2 +-
 arch/sh/kernel/ptrace_64.c  |  2 +-
 arch/sparc/kernel/ptrace_32.c   |  2 +-
 arch/sparc/kernel/ptrace_64.c   |  2 +-
 arch/um/kernel/ptrace.c |  2 +-
 arch/x86/entry/common.c |  2 +-
 arch/xtensa/kernel/ptrace.c |  2 +-
 include/linux/ptrace.h  | 16 ++---
 include/linux/tracehook.h   | 13 ++
 include/uapi/linux/ptrace.h | 22 +
 kernel/ptrace.c | 42 +
 29 files changed, 113 insertions(+), 32 deletions(-)

diff --git a/arch/alpha/kernel/ptrace.c b/arch/alpha/kernel/ptrace.c
index cb8d599e72d6..970c0719b4d1 100644
--- a/arch/alpha/kernel/ptrace.c
+++ b/arch/alpha/kernel/ptrace.c
@@ -324,7 +324,7 @@ asmlinkage unsigned long syscall_trace_enter(void)
unsigned long ret = 0;
struct pt_regs *regs = current_pt_regs();
if (test_thread_flag(TIF_SYSCALL_TRACE) &&
-   tracehook_report_syscall_entry(current_pt_regs()))
+   tracehook_report_syscall_entry(current_pt_regs(), false))
ret = -1UL;
audit_syscall_entry(regs->r0, regs->r16, regs->r17, regs->r18, 
regs->r19);

[RFC PATCH] ptrace: add PTRACE_GET_SYSCALL_INFO request

2018-11-06 Thread Elvira Khabirova
PTRACE_GET_SYSCALL_INFO lets ptracer obtain details of the syscall
the tracee is blocked in. The request returns meaningful data only
when the tracee is in a syscall-enter-stop or a syscall-exit-stop.

There are two reasons for a special syscall-related ptrace request.

Firstly, with the current ptrace API there are cases when ptracer cannot
retrieve necessary information about syscalls. Some examples include:
* The notorious int-0x80-from-64-bit-task issue. See [1] for details.
In short, if a 64-bit task performs a syscall through int 0x80, its tracer
has no reliable means to find out that the syscall was, in fact,
a compat syscall, and misidentifies it.
* Syscall-enter-stop and syscall-exit-stop look the same for the tracer.
Common practice is to keep track of the sequence of ptrace-stops in order
not to mix the two syscall-stops up. But it is not as simple as it looks;
for example, strace had a (just recently fixed) long-standing bug where
attaching strace to a tracee that is performing the execve system call
led to the tracer identifying the following syscall-exit-stop as
syscall-enter-stop, which messed up all the state tracking.
* Since the introduction of commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3
("ptrace: Don't allow accessing an undumpable mm"), both PTRACE_PEEKDATA
and process_vm_readv become unavailable when the process dumpable flag
is cleared. On ia64 this results in all syscall arguments being unavailable.

Secondly, ptracers also have to support a lot of arch-specific code for
obtaining information about the tracee. For some architectures, this
requires a ptrace(PTRACE_PEEKUSER, ...) invocation for every syscall
argument and return value.

PTRACE_GET_SYSCALL_INFO returns the following structure:

struct ptrace_syscall_info {
__u8 op; /* 0 for entry, 1 for exit */
__u8 __pad0[7];
union {
struct {
__u64 nr;
__u64 ip;
__u64 args[6];
__u8 is_compat;
__u8 __pad1[7];
} entry_info;
struct {
__s64 rval;
__u8 is_error;
__u8 __pad2[7];
} exit_info;
};
};

The structure was chosen according to [2], except for two changes.
First: instead of an arch field with a value of AUDIT_ARCH_*, a boolean
is_compat value is returned, because a) not all arches have an AUDIT_ARCH_*
defined for them, b) the tracer already knows what *arch* it is running on,
but it does not know whether the tracee/syscall is in compat mode or not.
Second: a boolean is_error value is added to rval. This way the tracer can
more reliably distinguish a return value from an error value.

[1] 
https://lkml.kernel.org/r/ca+55afzcsvmddj9lh_gdbz1ozhyem6zrgpbdajnywm2lf_e...@mail.gmail.com
[2] 
http://lkml.kernel.org/r/caobl_7gm0n80n7j_dfw_eqyflyzq+sf4y2avsccv88tb3aw...@mail.gmail.com

Signed-off-by: Elvira Khabirova 
---
 arch/alpha/kernel/ptrace.c  |  2 +-
 arch/arc/kernel/ptrace.c|  2 +-
 arch/arm/kernel/ptrace.c|  2 +-
 arch/arm64/kernel/ptrace.c  |  2 +-
 arch/c6x/kernel/ptrace.c|  2 +-
 arch/h8300/kernel/ptrace.c  |  2 +-
 arch/hexagon/kernel/traps.c |  2 +-
 arch/ia64/kernel/ptrace.c   |  2 +-
 arch/m68k/kernel/ptrace.c   |  3 ++-
 arch/microblaze/kernel/ptrace.c |  2 +-
 arch/mips/kernel/ptrace.c   |  2 +-
 arch/nds32/kernel/ptrace.c  |  2 +-
 arch/nios2/kernel/ptrace.c  |  3 ++-
 arch/openrisc/kernel/ptrace.c   |  2 +-
 arch/parisc/kernel/ptrace.c |  2 +-
 arch/powerpc/kernel/ptrace.c|  2 +-
 arch/riscv/kernel/ptrace.c  |  2 +-
 arch/s390/kernel/ptrace.c   |  2 +-
 arch/sh/kernel/ptrace_32.c  |  2 +-
 arch/sh/kernel/ptrace_64.c  |  2 +-
 arch/sparc/kernel/ptrace_32.c   |  2 +-
 arch/sparc/kernel/ptrace_64.c   |  2 +-
 arch/um/kernel/ptrace.c |  2 +-
 arch/x86/entry/common.c |  2 +-
 arch/xtensa/kernel/ptrace.c |  2 +-
 include/linux/ptrace.h  | 16 ++---
 include/linux/tracehook.h   | 13 ++
 include/uapi/linux/ptrace.h | 22 +
 kernel/ptrace.c | 42 +
 29 files changed, 113 insertions(+), 32 deletions(-)

diff --git a/arch/alpha/kernel/ptrace.c b/arch/alpha/kernel/ptrace.c
index cb8d599e72d6..970c0719b4d1 100644
--- a/arch/alpha/kernel/ptrace.c
+++ b/arch/alpha/kernel/ptrace.c
@@ -324,7 +324,7 @@ asmlinkage unsigned long syscall_trace_enter(void)
unsigned long ret = 0;
struct pt_regs *regs = current_pt_regs();
if (test_thread_flag(TIF_SYSCALL_TRACE) &&
-   tracehook_report_syscall_entry(current_pt_regs()))
+   tracehook_report_syscall_entry(current_pt_regs(), false))
ret = -1UL;
audit_syscall_entry(regs->r0, regs->r16, regs->r17, regs->r18, 
regs->r19);

Re: [PATCH] nbd-client: support newstyle protocol, -b, -d

2018-10-29 Thread Elvira Khabirova
On Wed, 24 Oct 2018 15:53:44 +0200
Denys Vlasenko  wrote:

> I applied a modified version of the patch,
> please let me know whether it works.
> Thanks!

It does, both on x86_64 and MIPS64be, with old and new servers.
Thank you!
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


Re: [PATCH] nbd-client: support newstyle protocol, -b, -d

2018-10-19 Thread Elvira Khabirova
On Tue, 9 Oct 2018 01:41:48 +0200
Elvira Khabirova  wrote:

> Recognize the "newstyle" protocol and switch to it automatically.
> Add options for setting blocksize (-b) and for disconnecting
> a nbd device (-d).

Ping. Support for oldstyle protocol was dropped in nbd-server v3.10,
which was released in 2015.  I tested this patch both with an old (3.8)
and a new (3.18) server.
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox


[PATCH] nbd-client: support newstyle protocol, -b, -d

2018-10-08 Thread Elvira Khabirova
Recognize the "newstyle" protocol and switch to it automatically.
Add options for setting blocksize (-b) and for disconnecting
a nbd device (-d).

Signed-off-by: Elvira Khabirova 
---
 networking/nbd-client.c | 168 +---
 1 file changed, 140 insertions(+), 28 deletions(-)

diff --git a/networking/nbd-client.c b/networking/nbd-client.c
index bedb01a1c..9e718ff93 100644
--- a/networking/nbd-client.c
+++ b/networking/nbd-client.c
@@ -27,17 +27,17 @@
 #define NBD_SET_SIZE_BLOCKS   _IO(0xab, 7)
 #define NBD_DISCONNECT_IO(0xab, 8)
 #define NBD_SET_TIMEOUT   _IO(0xab, 9)
+#define NBD_SET_FLAGS _IO(0xab, 10)
 
 //usage:#define nbdclient_trivial_usage
-//usage:   "HOST PORT BLOCKDEV"
+//usage:   "{ [-b BLKSIZE] [-N name] HOST PORT | -d } BLOCKDEV"
 //usage:#define nbdclient_full_usage "\n\n"
 //usage:   "Connect to HOST and provide a network block device on BLOCKDEV"
 
 //TODO: more compat with nbd-client version 2.9.13 -
 //Usage: nbd-client [bs=blocksize] [timeout=sec] host port nbd_device [-swap] 
[-persist] [-nofork]
-//Or   : nbd-client -d nbd_device
 //Or   : nbd-client -c nbd_device
-//Default value for blocksize is 1024 (recommended for ethernet)
+//Default value for blocksize is 4096
 //Allowed values for blocksize are 512,1024,2048,4096
 //Note, that kernel 2.4.2 and older ones do not work correctly with
 //blocksizes other than 1024 without patches
@@ -45,37 +45,90 @@
 int nbdclient_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int nbdclient_main(int argc UNUSED_PARAM, char **argv)
 {
-   unsigned long timeout = 0;
 #if BB_MMU
int nofork = 0;
 #endif
-   char *host, *port, *device;
+   char *host, *port, *device, *name;
+   unsigned blksize, size_blocks;
+   unsigned timeout;
+   unsigned opt;
+
+   uint16_t handshake_flags;
+   uint32_t client_flags = 0;
+
struct nbd_header_t {
uint64_t magic1; // "NBDMAGIC"
-   uint64_t magic2; // 0x420281861253 big endian
+   uint64_t magic2; // old style: 0x420281861253 big endian
+// new style: 0x49484156454F5054 (IHAVEOPT)
+   } nbd_header;
+
+   struct old_nbd_header_t {
uint64_t devsize;
uint32_t flags;
char data[124];
-   } nbd_header;
+   } old_nbd_header;
 
-   BUILD_BUG_ON(offsetof(struct nbd_header_t, data) != 8+8+8+4);
+   struct new_nbd_header_t {
+   uint64_t devsize;
+   uint16_t transmission_flags;
+   char data[124];
+   } new_nbd_header;
 
-   // Parse command line stuff (just a stub now)
-   if (!argv[1] || !argv[2] || !argv[3] || argv[4])
-   bb_show_usage();
+   struct nbd_opt_t {
+   uint64_t magic;
+   uint32_t opt;
+   uint32_t len;
+   } nbd_opts;
+
+   BUILD_BUG_ON(offsetof(struct old_nbd_header_t, data) != 8+4);
+   BUILD_BUG_ON(offsetof(struct new_nbd_header_t, data) != 8+2);
 
 #if !BB_MMU
bb_daemonize_or_rexec(DAEMON_CLOSE_EXTRA_FDS, argv);
 #endif
 
-   host = argv[1];
-   port = argv[2];
-   device = argv[3];
+   // Parse args
+   opt = getopt32(argv, "b:+t:+N:d", , , );
+   argv += optind;
+
+   if (opt & 0x8) {
+   if (argv[0]) {
+   int nbd = xopen(argv[0], O_RDWR);
+   ioctl(nbd, NBD_DISCONNECT);
+   ioctl(nbd, NBD_CLEAR_SOCK);
+   close(nbd);
+   return 0;
+   } else {
+   bb_show_usage();
+   }
+   }
+
+   if (!(opt & 0x1)) {
+   blksize = 4096;
+   }
+
+   if (!(opt & 0x2)) {
+   timeout = 0;
+   }
+
+   if (!(opt & 0x4)) {
+   name = NULL;
+   }
+
+   if (!argv[0] || !argv[1] || !argv[2] || argv[3]) {
+   bb_show_usage();
+   }
+
+   host = argv[0];
+   port = argv[1];
+   device = argv[2];
 
// Repeat until spanked (-persist behavior)
for (;;) {
int sock, nbd;
int ro;
+   int protover; // 0 for old, 1 for new
+   char *data;
 
// Make sure the /dev/nbd exists
nbd = xopen(device, O_RDWR);
@@ -85,25 +138,84 @@ int nbdclient_main(int argc UNUSED_PARAM, char **argv)
setsockopt_1(sock, IPPROTO_TCP, TCP_NODELAY);
 
// Log on to the server
-   xread(sock, _header, 8+8+8+4 + 124);
-   if (memcmp(_header.magic1, 
"NBDMAGIC""\x00\x00\x42\x02\x81\x86\x12\x53", 16) != 0)
+   xread(sock, _header, 8+8);
+   if (memcmp(_header.magic1, "NBDMAGIC",
+

[no subject]

2017-01-23 Thread Elvira Khabirova

-- 
debian-science-maintainers mailing list
debian-science-maintainers@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/debian-science-maintainers