Hi Tycho,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.17-rc5]
[cannot apply to next-20180517]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:    
https://github.com/0day-ci/linux/commits/Tycho-Andersen/seccomp-trap-to-userspace/20180519-071527
config: x86_64-randconfig-x010-201819 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   kernel/seccomp.c: In function '__seccomp_filter':
>> kernel/seccomp.c:891:46: warning: passing argument 2 of 
>> 'seccomp_do_user_notification' makes integer from pointer without a cast 
>> [-Wint-conversion]
      seccomp_do_user_notification(this_syscall, match, sd);
                                                 ^~~~~
   kernel/seccomp.c:802:13: note: expected 'u32 {aka unsigned int}' but 
argument is of type 'struct seccomp_filter *'
    static void seccomp_do_user_notification(int this_syscall,
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> kernel/seccomp.c:891:53: error: passing argument 3 of 
>> 'seccomp_do_user_notification' from incompatible pointer type 
>> [-Werror=incompatible-pointer-types]
      seccomp_do_user_notification(this_syscall, match, sd);
                                                        ^~
   kernel/seccomp.c:802:13: note: expected 'struct seccomp_filter *' but 
argument is of type 'const struct seccomp_data *'
    static void seccomp_do_user_notification(int this_syscall,
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> kernel/seccomp.c:891:3: error: too few arguments to function 
>> 'seccomp_do_user_notification'
      seccomp_do_user_notification(this_syscall, match, sd);
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/seccomp.c:802:13: note: declared here
    static void seccomp_do_user_notification(int this_syscall,
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/seccomp.c: In function 'seccomp_set_mode_filter':
>> kernel/seccomp.c:1036:14: error: implicit declaration of function 
>> 'get_unused_fd_flags'; did you mean 'getname_flags'? 
>> [-Werror=implicit-function-declaration]
      listener = get_unused_fd_flags(O_RDWR);
                 ^~~~~~~~~~~~~~~~~~~
                 getname_flags
>> kernel/seccomp.c:1042:16: error: implicit declaration of function 
>> 'init_listener'; did you mean 'init_llist_head'? 
>> [-Werror=implicit-function-declaration]
      listener_f = init_listener(current, prepared);
                   ^~~~~~~~~~~~~
                   init_llist_head
>> kernel/seccomp.c:1042:14: warning: assignment makes pointer from integer 
>> without a cast [-Wint-conversion]
      listener_f = init_listener(current, prepared);
                 ^
>> kernel/seccomp.c:1044:4: error: implicit declaration of function 
>> 'put_unused_fd'; did you mean 'put_user_ns'? 
>> [-Werror=implicit-function-declaration]
       put_unused_fd(listener);
       ^~~~~~~~~~~~~
       put_user_ns
>> kernel/seccomp.c:1077:4: error: implicit declaration of function 'fput'; did 
>> you mean 'iput'? [-Werror=implicit-function-declaration]
       fput(listener_f);
       ^~~~
       iput
>> kernel/seccomp.c:1080:4: error: implicit declaration of function 
>> 'fd_install'; did you mean 'fs_initcall'? 
>> [-Werror=implicit-function-declaration]
       fd_install(listener, listener_f);
       ^~~~~~~~~~
       fs_initcall
   cc1: some warnings being treated as errors

vim +/seccomp_do_user_notification +891 kernel/seccomp.c

   738  
   739  static void seccomp_do_user_notification(int this_syscall,
   740                                           struct seccomp_filter *match,
   741                                           const struct seccomp_data *sd)
   742  {
   743          int err;
   744          long ret = 0;
   745          struct seccomp_knotif n = {};
   746  
   747          mutex_lock(&match->notify_lock);
   748          if (!match->has_listener) {
   749                  err = -ENOSYS;
   750                  goto out;
   751          }
   752  
   753          n.pid = current->pid;
   754          n.state = SECCOMP_NOTIFY_INIT;
   755          n.data = sd;
   756          n.id = seccomp_next_notify_id(match);
   757          init_completion(&n.ready);
   758  
   759          list_add(&n.list, &match->notifications);
   760  
   761          mutex_unlock(&match->notify_lock);
   762          up(&match->request);
   763  
   764          err = wait_for_completion_interruptible(&n.ready);
   765          mutex_lock(&match->notify_lock);
   766  
   767          /*
   768           * Here it's possible we got a signal and then had to wait on 
the mutex
   769           * while the reply was sent, so let's be sure there wasn't a 
response
   770           * in the meantime.
   771           */
   772          if (err < 0 && n.state != SECCOMP_NOTIFY_REPLIED) {
   773                  /*
   774                   * We got a signal. Let's tell userspace about it 
(potentially
   775                   * again, if we had already notified them about the 
first one).
   776                   */
   777                  if (n.state == SECCOMP_NOTIFY_SENT) {
   778                          n.state = SECCOMP_NOTIFY_INIT;
   779                          up(&match->request);
   780                  }
   781                  mutex_unlock(&match->notify_lock);
   782                  err = wait_for_completion_killable(&n.ready);
   783                  mutex_lock(&match->notify_lock);
   784                  if (err < 0)
   785                          goto remove_list;
   786          }
   787  
   788          ret = n.val;
   789          err = n.error;
   790  
   791          WARN(n.state != SECCOMP_NOTIFY_REPLIED,
   792               "notified about write complete when state is not write");
   793  
   794  remove_list:
   795          list_del(&n.list);
   796  out:
   797          mutex_unlock(&match->notify_lock);
   798          syscall_set_return_value(current, task_pt_regs(current),
   799                                   err, ret);
   800  }
   801  #else
 > 802  static void seccomp_do_user_notification(int this_syscall,
   803                                           u32 action,
   804                                           struct seccomp_filter *match,
   805                                           const struct seccomp_data *sd)
   806  {
   807          WARN(1, "user notification received, but disabled");
   808          seccomp_log(this_syscall, SIGSYS, action, true);
   809          do_exit(SIGSYS);
   810  }
   811  #endif
   812  
   813  #ifdef CONFIG_SECCOMP_FILTER
   814  static int __seccomp_filter(int this_syscall, const struct seccomp_data 
*sd,
   815                              const bool recheck_after_trace)
   816  {
   817          u32 filter_ret, action;
   818          struct seccomp_filter *match = NULL;
   819          int data;
   820  
   821          /*
   822           * Make sure that any changes to mode from another thread have
   823           * been seen after TIF_SECCOMP was seen.
   824           */
   825          rmb();
   826  
   827          filter_ret = seccomp_run_filters(sd, &match);
   828          data = filter_ret & SECCOMP_RET_DATA;
   829          action = filter_ret & SECCOMP_RET_ACTION_FULL;
   830  
   831          switch (action) {
   832          case SECCOMP_RET_ERRNO:
   833                  /* Set low-order bits as an errno, capped at MAX_ERRNO. 
*/
   834                  if (data > MAX_ERRNO)
   835                          data = MAX_ERRNO;
   836                  syscall_set_return_value(current, task_pt_regs(current),
   837                                           -data, 0);
   838                  goto skip;
   839  
   840          case SECCOMP_RET_TRAP:
   841                  /* Show the handler the original registers. */
   842                  syscall_rollback(current, task_pt_regs(current));
   843                  /* Let the filter pass back 16 bits of data. */
   844                  seccomp_send_sigsys(this_syscall, data);
   845                  goto skip;
   846  
   847          case SECCOMP_RET_TRACE:
   848                  /* We've been put in this state by the ptracer already. 
*/
   849                  if (recheck_after_trace)
   850                          return 0;
   851  
   852                  /* ENOSYS these calls if there is no tracer attached. */
   853                  if (!ptrace_event_enabled(current, 
PTRACE_EVENT_SECCOMP)) {
   854                          syscall_set_return_value(current,
   855                                                   task_pt_regs(current),
   856                                                   -ENOSYS, 0);
   857                          goto skip;
   858                  }
   859  
   860                  /* Allow the BPF to provide the event message */
   861                  ptrace_event(PTRACE_EVENT_SECCOMP, data);
   862                  /*
   863                   * The delivery of a fatal signal during event
   864                   * notification may silently skip tracer notification,
   865                   * which could leave us with a potentially unmodified
   866                   * syscall that the tracer would have liked to have
   867                   * changed. Since the process is about to die, we just
   868                   * force the syscall to be skipped and let the signal
   869                   * kill the process and correctly handle any tracer exit
   870                   * notifications.
   871                   */
   872                  if (fatal_signal_pending(current))
   873                          goto skip;
   874                  /* Check if the tracer forced the syscall to be 
skipped. */
   875                  this_syscall = syscall_get_nr(current, 
task_pt_regs(current));
   876                  if (this_syscall < 0)
   877                          goto skip;
   878  
   879                  /*
   880                   * Recheck the syscall, since it may have changed. This
   881                   * intentionally uses a NULL struct seccomp_data to 
force
   882                   * a reload of all registers. This does not goto skip 
since
   883                   * a skip would have already been reported.
   884                   */
   885                  if (__seccomp_filter(this_syscall, NULL, true))
   886                          return -1;
   887  
   888                  return 0;
   889  
   890          case SECCOMP_RET_USER_NOTIF:
 > 891                  seccomp_do_user_notification(this_syscall, match, sd);
   892                  goto skip;
   893          case SECCOMP_RET_LOG:
   894                  seccomp_log(this_syscall, 0, action, true);
   895                  return 0;
   896  
   897          case SECCOMP_RET_ALLOW:
   898                  /*
   899                   * Note that the "match" filter will always be NULL for
   900                   * this action since SECCOMP_RET_ALLOW is the starting
   901                   * state in seccomp_run_filters().
   902                   */
   903                  return 0;
   904  
   905          case SECCOMP_RET_KILL_THREAD:
   906          case SECCOMP_RET_KILL_PROCESS:
   907          default:
   908                  seccomp_log(this_syscall, SIGSYS, action, true);
   909                  /* Dump core only if this is the last remaining thread. 
*/
   910                  if (action == SECCOMP_RET_KILL_PROCESS ||
   911                      get_nr_threads(current) == 1) {
   912                          siginfo_t info;
   913  
   914                          /* Show the original registers in the dump. */
   915                          syscall_rollback(current, 
task_pt_regs(current));
   916                          /* Trigger a manual coredump since do_exit 
skips it. */
   917                          seccomp_init_siginfo(&info, this_syscall, data);
   918                          do_coredump(&info);
   919                  }
   920                  if (action == SECCOMP_RET_KILL_PROCESS)
   921                          do_group_exit(SIGSYS);
   922                  else
   923                          do_exit(SIGSYS);
   924          }
   925  
   926          unreachable();
   927  
   928  skip:
   929          seccomp_log(this_syscall, 0, action, match ? match->log : 
false);
   930          return -1;
   931  }
   932  #else
   933  static int __seccomp_filter(int this_syscall, const struct seccomp_data 
*sd,
   934                              const bool recheck_after_trace)
   935  {
   936          BUG();
   937  }
   938  #endif
   939  
   940  int __secure_computing(const struct seccomp_data *sd)
   941  {
   942          int mode = current->seccomp.mode;
   943          int this_syscall;
   944  
   945          if (IS_ENABLED(CONFIG_CHECKPOINT_RESTORE) &&
   946              unlikely(current->ptrace & PT_SUSPEND_SECCOMP))
   947                  return 0;
   948  
   949          this_syscall = sd ? sd->nr :
   950                  syscall_get_nr(current, task_pt_regs(current));
   951  
   952          switch (mode) {
   953          case SECCOMP_MODE_STRICT:
   954                  __secure_computing_strict(this_syscall);  /* may call 
do_exit */
   955                  return 0;
   956          case SECCOMP_MODE_FILTER:
   957                  return __seccomp_filter(this_syscall, sd, false);
   958          default:
   959                  BUG();
   960          }
   961  }
   962  #endif /* CONFIG_HAVE_ARCH_SECCOMP_FILTER */
   963  
   964  long prctl_get_seccomp(void)
   965  {
   966          return current->seccomp.mode;
   967  }
   968  
   969  /**
   970   * seccomp_set_mode_strict: internal function for setting strict seccomp
   971   *
   972   * Once current->seccomp.mode is non-zero, it may not be changed.
   973   *
   974   * Returns 0 on success or -EINVAL on failure.
   975   */
   976  static long seccomp_set_mode_strict(void)
   977  {
   978          const unsigned long seccomp_mode = SECCOMP_MODE_STRICT;
   979          long ret = -EINVAL;
   980  
   981          spin_lock_irq(&current->sighand->siglock);
   982  
   983          if (!seccomp_may_assign_mode(seccomp_mode))
   984                  goto out;
   985  
   986  #ifdef TIF_NOTSC
   987          disable_TSC();
   988  #endif
   989          seccomp_assign_mode(current, seccomp_mode);
   990          ret = 0;
   991  
   992  out:
   993          spin_unlock_irq(&current->sighand->siglock);
   994  
   995          return ret;
   996  }
   997  
   998  #ifdef CONFIG_SECCOMP_FILTER
   999  #ifdef CONFIG_SECCOMP_USER_NOTIFICATION
  1000  static struct file *init_listener(struct task_struct *,
  1001                                    struct seccomp_filter *);
  1002  #endif
  1003  
  1004  /**
  1005   * seccomp_set_mode_filter: internal function for setting seccomp filter
  1006   * @flags:  flags to change filter behavior
  1007   * @filter: struct sock_fprog containing filter
  1008   *
  1009   * This function may be called repeatedly to install additional filters.
  1010   * Every filter successfully installed will be evaluated (in reverse 
order)
  1011   * for each system call the task makes.
  1012   *
  1013   * Once current->seccomp.mode is non-zero, it may not be changed.
  1014   *
  1015   * Returns 0 on success or -EINVAL on failure.
  1016   */
  1017  static long seccomp_set_mode_filter(unsigned int flags,
  1018                                      const char __user *filter)
  1019  {
  1020          const unsigned long seccomp_mode = SECCOMP_MODE_FILTER;
  1021          struct seccomp_filter *prepared = NULL;
  1022          long ret = -EINVAL;
  1023          int listener = 0;
  1024          struct file *listener_f = NULL;
  1025  
  1026          /* Validate flags. */
  1027          if (flags & ~SECCOMP_FILTER_FLAG_MASK)
  1028                  return -EINVAL;
  1029  
  1030          /* Prepare the new filter before holding any locks. */
  1031          prepared = seccomp_prepare_user_filter(filter);
  1032          if (IS_ERR(prepared))
  1033                  return PTR_ERR(prepared);
  1034  
  1035          if (flags & SECCOMP_FILTER_FLAG_GET_LISTENER) {
> 1036                  listener = get_unused_fd_flags(O_RDWR);
  1037                  if (listener < 0) {
  1038                          ret = listener;
  1039                          goto out_free;
  1040                  }
  1041  
> 1042                  listener_f = init_listener(current, prepared);
  1043                  if (IS_ERR(listener_f)) {
> 1044                          put_unused_fd(listener);
  1045                          ret = PTR_ERR(listener_f);
  1046                          goto out_free;
  1047                  }
  1048          }
  1049  
  1050          /*
  1051           * Make sure we cannot change seccomp or nnp state via TSYNC
  1052           * while another thread is in the middle of calling exec.
  1053           */
  1054          if (flags & SECCOMP_FILTER_FLAG_TSYNC &&
  1055              mutex_lock_killable(&current->signal->cred_guard_mutex))
  1056                  goto out_put_fd;
  1057  
  1058          spin_lock_irq(&current->sighand->siglock);
  1059  
  1060          if (!seccomp_may_assign_mode(seccomp_mode))
  1061                  goto out;
  1062  
  1063          ret = seccomp_attach_filter(flags, prepared);
  1064          if (ret)
  1065                  goto out;
  1066          /* Do not free the successfully attached filter. */
  1067          prepared = NULL;
  1068  
  1069          seccomp_assign_mode(current, seccomp_mode);
  1070  out:
  1071          spin_unlock_irq(&current->sighand->siglock);
  1072          if (flags & SECCOMP_FILTER_FLAG_TSYNC)
  1073                  mutex_unlock(&current->signal->cred_guard_mutex);
  1074  out_put_fd:
  1075          if (flags & SECCOMP_FILTER_FLAG_GET_LISTENER) {
  1076                  if (ret < 0) {
> 1077                          fput(listener_f);
  1078                          put_unused_fd(listener);
  1079                  } else {
> 1080                          fd_install(listener, listener_f);
  1081                          ret = listener;
  1082                  }
  1083          }
  1084  out_free:
  1085          seccomp_filter_free(prepared);
  1086          return ret;
  1087  }
  1088  #else
  1089  static inline long seccomp_set_mode_filter(unsigned int flags,
  1090                                             const char __user *filter)
  1091  {
  1092          return -EINVAL;
  1093  }
  1094  #endif
  1095  

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Attachment: .config.gz
Description: application/gzip

Reply via email to