Re: [PATCH v2 1/4] seccomp: add a return code to trap to userspace

From: kbuild test robot
Date: Fri May 18 2018 - 19:18:31 EST


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