| Message ID | 20180901100026.16956-1-baijiaju1990@gmail.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: scm: Fix a possible sleep-in-atomic-context bug in scm_fp_copy() | expand |
From: Jia-Ju Bai <baijiaju1990@gmail.com> Date: Sat, 1 Sep 2018 18:00:26 +0800 > The kernel module may sleep with holding a spinlock. > > The function call paths (from bottom to top) in Linux-4.16 are: > > [FUNC] kmalloc(GFP_KERNEL) > net/core/scm.c, 85: kmalloc in scm_fp_copy > net/core/scm.c, 161: scm_fp_copy in __scm_send > ./include/net/scm.h, 88: __scm_send in scm_send > net/unix/af_unix.c, 1600: scm_send in maybe_init_creds > net/unix/af_unix.c, 1983: maybe_init_creds in unix_stream_sendpage > net/unix/af_unix.c, 1973: spin_lock in unix_stream_sendpage Please, do a full analysis of the code for these changes you are submitting. Read maybe_init_creds(), it sets msg.msg_controllen to zero. struct msghdr msg = { .msg_controllen = 0 }; When that is zero, __scm__send() is never called. static __inline__ int scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *scm, bool forcecreds) { ... if (msg->msg_controllen <= 0) return 0; return __scm_send(sock, msg, scm); If this bug existed, sleeping in atomic warnings would be triggering all the time and people would report that.
Thanks for your reply. On 2018/9/3 7:01, David Miller wrote: > From: Jia-Ju Bai <baijiaju1990@gmail.com> > Date: Sat, 1 Sep 2018 18:00:26 +0800 > >> The kernel module may sleep with holding a spinlock. >> >> The function call paths (from bottom to top) in Linux-4.16 are: >> >> [FUNC] kmalloc(GFP_KERNEL) >> net/core/scm.c, 85: kmalloc in scm_fp_copy >> net/core/scm.c, 161: scm_fp_copy in __scm_send >> ./include/net/scm.h, 88: __scm_send in scm_send >> net/unix/af_unix.c, 1600: scm_send in maybe_init_creds >> net/unix/af_unix.c, 1983: maybe_init_creds in unix_stream_sendpage >> net/unix/af_unix.c, 1973: spin_lock in unix_stream_sendpage > Please, do a full analysis of the code for these changes you are > submitting. > > Read maybe_init_creds(), it sets msg.msg_controllen to zero. > > struct msghdr msg = { .msg_controllen = 0 }; > > When that is zero, __scm__send() is never called. Oh, I did not notice this, sorry... > static __inline__ int scm_send(struct socket *sock, struct msghdr *msg, > struct scm_cookie *scm, bool forcecreds) > { > ... > if (msg->msg_controllen <= 0) > return 0; > return __scm_send(sock, msg, scm); > > If this bug existed, sleeping in atomic warnings would be triggering > all the time and people would report that. Sorry for this false positive. I will check the code more carefully before submitting my patches. Best wishes, Jia-Ju Bai
diff --git a/net/core/scm.c b/net/core/scm.c index b1ff8a441748..49548f81c45b 100644 --- a/net/core/scm.c +++ b/net/core/scm.c @@ -82,7 +82,7 @@ static int scm_fp_copy(struct cmsghdr *cmsg, struct scm_fp_list **fplp) if (!fpl) { - fpl = kmalloc(sizeof(struct scm_fp_list), GFP_KERNEL); + fpl = kmalloc(sizeof(struct scm_fp_list), GFP_ATOMIC); if (!fpl) return -ENOMEM; *fplp = fpl;
The kernel module may sleep with holding a spinlock. The function call paths (from bottom to top) in Linux-4.16 are: [FUNC] kmalloc(GFP_KERNEL) net/core/scm.c, 85: kmalloc in scm_fp_copy net/core/scm.c, 161: scm_fp_copy in __scm_send ./include/net/scm.h, 88: __scm_send in scm_send net/unix/af_unix.c, 1600: scm_send in maybe_init_creds net/unix/af_unix.c, 1983: maybe_init_creds in unix_stream_sendpage net/unix/af_unix.c, 1973: spin_lock in unix_stream_sendpage To fix this bug, GFP_KERNEL is replaced with GFP_ATOMIC. This bug is found by my static analysis tool DSAC. Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com> --- net/core/scm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)