3.18.84-stable review

This is the start of the stable review cycle for the 3.18.84 release. There are 12 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.

Responses should be made by Fri Nov 24 10:10:45 UTC 2017. Anything received after that time might be too late.

The whole patch series can be found in one patch at:

	kernel.org/pub/linux/kernel/v3.x/stable-review/patch-3.18.84-rc1.gz

or in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-3.18.y
and the diffstat can be found below.

thanks,

greg k-h

-------------
Pseudo-Shortlog of commits:

Greg Kroah-Hartman <[email protected]>

    Linux 3.18.84-rc1

Jan Harkes <[email protected]>
coda: fix 'kernel memory exposure attempt' in fsync

Corey Minyard <[email protected]>
ipmi: fix unsigned long underflow

alex chen <[email protected]>
ocfs2: should wait dio before inode lock in ocfs2_setattr()

Roberto Sassu <[email protected]>
ima: do not update security.ima if appraisal status is not INTEGRITY_PASS

Cong Wang <[email protected]>
vlan: fix a use-after-free in vlan_device_event()

Jason A. Donenfeld <[email protected]>
af_netlink: ensure that NLMSG_DONE never fails in dumps

Huacai Chen <[email protected]>
fealnx: Fix building error on MIPS

Xin Long <[email protected]>
sctp: do not peel off an assoc from one netns to another one

Ye Yin <[email protected]>
netfilter/ipvs: clear ipvs_property flag when SKB net namespace changed

Eric Dumazet <[email protected]>

    tcp: do not mangle skb->cb[] in tcp_make_synack()

Eric W. Biederman <[email protected]>
net/sctp: Always set scope_id in sctp_inet6_skb_msgname

WANG Cong <[email protected]>
ipv6/dccp: do not inherit ipv6_mc_list from parent

-------------

Diffstat:

 Makefile                              |  4 ++--
 drivers/char/ipmi/ipmi_msghandler.c   | 10 ++++++----
 drivers/net/ethernet/fealnx.c         |  6 +++---
 fs/coda/upcall.c                      |  3 +--
 fs/ocfs2/file.c                       |  9 +++++++--
 include/linux/skbuff.h                |  7 +++++++
 net/8021q/vlan.c                      |  6 +++---
 net/core/skbuff.c                     |  1 +
 net/dccp/ipv6.c                       |  7 +++++++
 net/ipv4/tcp_output.c                 |  9 ++-------
 net/ipv6/tcp_ipv6.c                   |  2 ++
 net/netlink/af_netlink.c              | 17 +++++++++++------
 net/netlink/af_netlink.h              |  1 +
 net/sctp/ipv6.c                       |  2 ++
 net/sctp/socket.c                     |  4 ++++
 security/integrity/ima/ima_appraise.c |  3 +++
 16 files changed, 62 insertions(+), 29 deletions(-)

3.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: WANG Cong <[email protected]>

commit 83eaddab4378db256d00d295bda6ca997cd13a52 upstream.

Like commit 657831ffc38e ("dccp/tcp: do not inherit mc_list from parent") we should clear ipv6_mc_list etc. for IPv6 sockets too.

---
 net/dccp/ipv6.c     |    7 +++++++
 net/ipv6/tcp_ipv6.c |    2 ++
 2 files changed, 9 insertions(+)

@@ -487,6 +487,9 @@ static struct sock *dccp_v6_request_recv
 		newsk->sk_backlog_rcv = dccp_v4_do_rcv;
 		newnp->pktoptions  = NULL;
 		newnp->opt	   = NULL;
+		newnp->ipv6_mc_list = NULL;
+		newnp->ipv6_ac_list = NULL;
+		newnp->ipv6_fl_list = NULL;
 		newnp->mcast_oif   = inet6_iif(skb);
 		newnp->mcast_hops  = ipv6_hdr(skb)->hop_limit;
 

@@ -562,6 +565,10 @@ static struct sock *dccp_v6_request_recv
 	/* Clone RX bits */
 	newnp->rxopt.all = np->rxopt.all;
 
+	newnp->ipv6_mc_list = NULL;
+	newnp->ipv6_ac_list = NULL;
+	newnp->ipv6_fl_list = NULL;
+
 	/* Clone pktoptions received with SYN */
 	newnp->pktoptions = NULL;
 	if (ireq->pktopts != NULL) {

@@ -1113,6 +1113,7 @@ static struct sock *tcp_v6_syn_recv_sock
 		newtp->af_specific = &tcp_sock_ipv6_mapped_specific;
 #endif
 
+		newnp->ipv6_mc_list = NULL;
 		newnp->ipv6_ac_list = NULL;
 		newnp->ipv6_fl_list = NULL;
 		newnp->pktoptions  = NULL;

@@ -1184,6 +1185,7 @@ static struct sock *tcp_v6_syn_recv_sock
 	   First: no IPv4 options.
 	 */
 	newinet->inet_opt = NULL;
+	newnp->ipv6_mc_list = NULL;
 	newnp->ipv6_ac_list = NULL;
 	newnp->ipv6_fl_list = NULL;
 

3.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: "Eric W. Biederman" <[email protected]>

[ Upstream commit 7c8a61d9ee1df0fb4747879fa67a99614eb62fec ]

Alexandar Potapenko while testing the kernel with KMSAN and syzkaller discovered that in some configurations sctp would leak 4 bytes of kernel stack.

Working with his reproducer I discovered that those 4 bytes that are leaked is the scope id of an ipv6 address returned by recvmsg.

With a little code inspection and a shrewd guess I discovered that sctp_inet6_skb_msgname only initializes the scope_id field for link local ipv6 addresses to the interface index the link local address pertains to instead of initializing the scope_id field for all ipv6 addresses.

That is almost reasonable as scope_id's are meaniningful only for link local addresses. Set the scope_id in all other cases to 0 which is not a valid interface index to make it clear there is nothing useful in the scope_id field.

There should be no danger of breaking userspace as the stack leak guaranteed that previously meaningless random data was being returned.

History-tree: https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git

---
 net/sctp/ipv6.c |    2 ++
 1 file changed, 2 insertions(+)

@@ -805,6 +805,8 @@ static void sctp_inet6_skb_msgname(struc
 		if (ipv6_addr_type(&addr->v6.sin6_addr) & IPV6_ADDR_LINKLOCAL) {
 			struct sctp_ulpevent *ev = sctp_skb2event(skb);
 			addr->v6.sin6_scope_id = ev->iif;
+		} else {
+			addr->v6.sin6_scope_id = 0;
 		}
 	}
 

3.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Eric Dumazet <[email protected]>

[ Upstream commit 3b11775033dc87c3d161996c54507b15ba26414a ]

Christoph Paasch sent a patch to address the following issue :

tcp_make_synack() is leaving some TCP private info in skb->cb[], then send the packet by other means than tcp_transmit_skb()

tcp_transmit_skb() makes sure to clear skb->cb[] to not confuse IPv4/IPV6 stacks, but we have no such cleanup for SYNACK.

tcp_make_synack() should not use tcp_init_nondata_skb() :

tcp_init_nondata_skb() really should be limited to skbs put in write/rtx queues (the ones that are only sent via tcp_transmit_skb())

This patch fixes the issue and should even save few cpu cycles ;)

---
 net/ipv4/tcp_output.c |    9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

@@ -2911,13 +2911,8 @@ struct sk_buff *tcp_make_synack(struct s
 	tcp_ecn_make_synack(req, th, sk);
 	th->source = htons(ireq->ir_num);
 	th->dest = ireq->ir_rmt_port;
-	/* Setting of flags are superfluous here for callers (and ECE is
-	 * not even correctly set)
-	 */
-	tcp_init_nondata_skb(skb, tcp_rsk(req)->snt_isn,
-			     TCPHDR_SYN | TCPHDR_ACK);
-
-	th->seq = htonl(TCP_SKB_CB(skb)->seq);
+	skb->ip_summed = CHECKSUM_PARTIAL;
+	th->seq = htonl(tcp_rsk(req)->snt_isn);
 	/* XXX data is queued and acked as is. No buffer/window check */
 	th->ack_seq = htonl(tcp_rsk(req)->rcv_nxt);
 

3.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Ye Yin <[email protected]>

[ Upstream commit 2b5ec1a5f9738ee7bf8f5ec0526e75e00362c48f ]

When run ipvs in two different network namespace at the same host, and one ipvs transport network traffic to the other network namespace ipvs. 'ipvs_property' flag will make the second ipvs take no effect. So we should clear 'ipvs_property' when SKB network namespace changed.

---
 include/linux/skbuff.h |    7 +++++++
 net/core/skbuff.c      |    1 +
 2 files changed, 8 insertions(+)

@@ -3117,6 +3117,13 @@ static inline void nf_reset_trace(struct
 #endif
 }
 
+static inline void ipvs_reset(struct sk_buff *skb)
+{
+#if IS_ENABLED(CONFIG_IP_VS)
+	skb->ipvs_property = 0;
+#endif
+}
+
 /* Note: This doesn't put any conntrack and bridge info in dst. */
 static inline void __nf_copy(struct sk_buff *dst, const struct sk_buff *src,
 			     bool copy)

@@ -4069,6 +4069,7 @@ void skb_scrub_packet(struct sk_buff *sk
 	if (!xnet)
 		return;
 
+	ipvs_reset(skb);
 	skb_orphan(skb);
 	skb->mark = 0;
 }

3.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Xin Long <[email protected]>

[ Upstream commit df80cd9b28b9ebaa284a41df611dbf3a2d05ca74 ]

Now when peeling off an association to the sock in another netns, all transports in this assoc are not to be rehashed and keep use the old key in hashtable.

As a transport uses sk->net as the hash key to insert into hashtable, it would miss removing these transports from hashtable due to the new netns when closing the sock and all transports are being freeed, then later an use-after-free issue could be caused when looking up an asoc and dereferencing those transports.

This is a very old issue since very beginning, ChunYu found it with syzkaller fuzz testing with this series:

socket$inet6_sctp()
bind$inet6()
sendto$inet6()
unshare(0x40000000)
getsockopt$inet_sctp6_SCTP_GET_ASSOC_ID_LIST()
getsockopt$inet_sctp6_SCTP_SOCKOPT_PEELOFF()

This patch is to block this call when peeling one assoc off from one netns to another one, so that the netns of all transport would not go out-sync with the key in hashtable.

Note that this patch didn't fix it by rehashing transports, as it's difficult to handle the situation when the tuple is already in use in the new netns. Besides, no one would like to peel off one assoc to another netns, considering ipaddrs, ifaces, etc. are usually different.

---
 net/sctp/socket.c |    4 ++++
 1 file changed, 4 insertions(+)

@@ -4466,6 +4466,10 @@ int sctp_do_peeloff(struct sock *sk, sct
 	struct socket *sock;
 	int err = 0;
 
+	/* Do not peel off from one netns to another one. */
+	if (!net_eq(current->nsproxy->net_ns, sock_net(sk)))
+		return -EINVAL;
+
 	if (!asoc)
 		return -EINVAL;
 

3.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Huacai Chen <[email protected]>

[ Upstream commit cc54c1d32e6a4bb3f116721abf900513173e4d02 ]

This patch try to fix the building error on MIPS. The reason is MIPS has already defined the LONG macro, which conflicts with the LONG enum in drivers/net/ethernet/fealnx.c.

---
 drivers/net/ethernet/fealnx.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

@@ -257,8 +257,8 @@ enum rx_desc_status_bits {
 	RXFSD = 0x00000800,	/* first descriptor */
 	RXLSD = 0x00000400,	/* last descriptor */
 	ErrorSummary = 0x80,	/* error summary */
-	RUNT = 0x40,		/* runt packet received */
-	LONG = 0x20,		/* long packet received */
+	RUNTPKT = 0x40,		/* runt packet received */
+	LONGPKT = 0x20,		/* long packet received */
 	FAE = 0x10,		/* frame align error */
 	CRC = 0x08,		/* crc error */
 	RXER = 0x04,		/* receive error */

@@ -1633,7 +1633,7 @@ static int netdev_rx(struct net_device *
 					       dev->name, rx_status);
 
 				dev->stats.rx_errors++;	/* end of a packet. */
-				if (rx_status & (LONG | RUNT))
+				if (rx_status & (LONGPKT | RUNTPKT))
 					dev->stats.rx_length_errors++;
 				if (rx_status & RXER)
 					dev->stats.rx_frame_errors++;

3.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: "Jason A. Donenfeld" <[email protected]>

[ Upstream commit 0642840b8bb008528dbdf929cec9f65ac4231ad0 ]

The way people generally use netlink_dump is that they fill in the skb as much as possible, breaking when nla_put returns an error. Then, they get called again and start filling out the next skb, and again, and so forth. The mechanism at work here is the ability for the iterative dumping function to detect when the skb is filled up and not fill it past the brim, waiting for a fresh skb for the rest of the data.

However, if the attributes are small and nicely packed, it is possible that a dump callback function successfully fills in attributes until the skb is of size 4080 (libmnl's default page-sized receive buffer size). The dump function completes, satisfied, and then, if it happens to be that this is actually the last skb, and no further ones are to be sent, then netlink_dump will add on the NLMSG_DONE part:

  nlh = nlmsg_put_answer(skb, cb, NLMSG_DONE, sizeof(len), NLM_F_MULTI);

It is very important that netlink_dump does this, of course. However, in this example, that call to nlmsg_put_answer will fail, because the previous filling by the dump function did not leave it enough room. And how could it possibly have done so? All of the nla_put variety of functions simply check to see if the skb has enough tailroom, independent of the context it is in.

In order to keep the important assumptions of all netlink dump users, it is therefore important to give them an skb that has this end part of the tail already reserved, so that the call to nlmsg_put_answer does not fail. Otherwise, library authors are forced to find some bizarre sized receive buffer that has a large modulo relative to the common sizes of messages received, which is ugly and buggy.

This patch thus saves the NLMSG_DONE for an additional message, for the case that things are dangerously close to the brim. This requires keeping track of the errno from ->dump() across calls.

---
 net/netlink/af_netlink.c |   17 +++++++++++------
 net/netlink/af_netlink.h |    1 +
 2 files changed, 12 insertions(+), 6 deletions(-)

@@ -1927,7 +1927,7 @@ static int netlink_dump(struct sock *sk)
 	struct sk_buff *skb = NULL;
 	struct nlmsghdr *nlh;
 	struct module *module;
-	int len, err = -ENOBUFS;
+	int err = -ENOBUFS;
 	int alloc_size;
 
 	mutex_lock(nlk->cb_mutex);

@@ -1965,9 +1965,11 @@ static int netlink_dump(struct sock *sk)
 		goto errout_skb;
 	netlink_skb_set_owner_r(skb, sk);
 
-	len = cb->dump(skb, cb);
+	if (nlk->dump_done_errno > 0)
+		nlk->dump_done_errno = cb->dump(skb, cb);
 
-	if (len > 0) {
+	if (nlk->dump_done_errno > 0 ||
+	    skb_tailroom(skb) < nlmsg_total_size(sizeof(nlk->dump_done_errno))) {
 		mutex_unlock(nlk->cb_mutex);
 
 		if (sk_filter(sk, skb))

@@ -1977,13 +1979,15 @@ static int netlink_dump(struct sock *sk)
 		return 0;
 	}
 
-	nlh = nlmsg_put_answer(skb, cb, NLMSG_DONE, sizeof(len), NLM_F_MULTI);
-	if (!nlh)
+	nlh = nlmsg_put_answer(skb, cb, NLMSG_DONE,
+			       sizeof(nlk->dump_done_errno), NLM_F_MULTI);
+	if (WARN_ON(!nlh))
 		goto errout_skb;
 
 	nl_dump_check_consistent(cb, nlh);
 
-	memcpy(nlmsg_data(nlh), &len, sizeof(len));
+	memcpy(nlmsg_data(nlh), &nlk->dump_done_errno,
+	       sizeof(nlk->dump_done_errno));
 
 	if (sk_filter(sk, skb))
 		kfree_skb(skb);

@@ -2048,6 +2052,7 @@ int __netlink_dump_start(struct sock *ss
 	cb->skb = skb;
 
 	nlk->cb_running = true;
+	nlk->dump_done_errno = INT_MAX;
 
 	mutex_unlock(nlk->cb_mutex);
 

@@ -35,6 +35,7 @@ struct netlink_sock {
 	size_t			max_recvmsg_len;
 	wait_queue_head_t	wait;
 	bool			cb_running;
+	int			dump_done_errno;
 	struct netlink_callback	cb;
 	struct mutex		*cb_mutex;
 	struct mutex		cb_def_mutex;

3.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Cong Wang <[email protected]>

[ Upstream commit 052d41c01b3a2e3371d66de569717353af489d63 ]

After refcnt reaches zero, vlan_vid_del() could free dev->vlan_info via RCU:

	RCU_INIT_POINTER(dev->vlan_info, NULL);
	call_rcu(&vlan_info->rcu, vlan_info_rcu_free);

However, the pointer 'grp' still points to that memory since it is set before vlan_vid_del():

        vlan_info = rtnl_dereference(dev->vlan_info);
        if (!vlan_info)
                goto out;
        grp = &vlan_info->grp;

Depends on when that RCU callback is scheduled, we could trigger a use-after-free in vlan_group_for_each_dev() right following this vlan_vid_del().

Fix it by moving vlan_vid_del() before setting grp. This is also symmetric to the vlan_vid_add() we call in vlan_device_event().

---
 net/8021q/vlan.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

@@ -376,6 +376,9 @@ static int vlan_device_event(struct noti
 			dev->name);
 		vlan_vid_add(dev, htons(ETH_P_8021Q), 0);
 	}
+	if (event == NETDEV_DOWN &&
+	    (dev->features & NETIF_F_HW_VLAN_CTAG_FILTER))
+		vlan_vid_del(dev, htons(ETH_P_8021Q), 0);
 
 	vlan_info = rtnl_dereference(dev->vlan_info);
 	if (!vlan_info)

@@ -420,9 +423,6 @@ static int vlan_device_event(struct noti
 		break;
 
 	case NETDEV_DOWN:
-		if (dev->features & NETIF_F_HW_VLAN_CTAG_FILTER)
-			vlan_vid_del(dev, htons(ETH_P_8021Q), 0);
-
 		/* Put all VLANs for this dev in the down state too.  */
 		vlan_group_for_each_dev(grp, i, vlandev) {
 			flgs = vlandev->flags;

3.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Roberto Sassu <[email protected]>

commit 020aae3ee58c1af0e7ffc4e2cc9fe4dc630338cb upstream.

Commit b65a9cfc2c38 ("Untangling ima mess, part 2: deal with counters") moved the call of ima_file_check() from may_open() to do_filp_open() at a point where the file descriptor is already opened.

This breaks the assumption made by IMA that file descriptors being closed belong to files whose access was granted by ima_file_check(). The consequence is that security.ima and security.evm are updated with good values, regardless of the current appraisal status.

For example, if a file does not have security.ima, IMA will create it after opening the file for writing, even if access is denied. Access to the file will be allowed afterwards.

Avoid this issue by checking the appraisal status before updating security.ima.

---
 security/integrity/ima/ima_appraise.c |    3 +++
 1 file changed, 3 insertions(+)

@@ -297,6 +297,9 @@ void ima_update_xattr(struct integrity_i
 	if (iint->flags & IMA_DIGSIG)
 		return;
 
+	if (iint->ima_file_status != INTEGRITY_PASS)
+		return;
+
 	rc = ima_collect_measurement(iint, file, NULL, NULL);
 	if (rc < 0)
 		return;

3.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: alex chen <[email protected]>

commit 28f5a8a7c033cbf3e32277f4cc9c6afd74f05300 upstream.

we should wait dio requests to finish before inode lock in ocfs2_setattr(), otherwise the following deadlock will happen:

process 1                  process 2                    process 3
truncate file 'A'          end_io of writing file 'A'   receiving the bast messages

ocfs2_setattr
ocfs2_inode_lock_tracker
ocfs2_inode_lock_full
inode_dio_wait
__inode_dio_wait
-->waiting for all dio
requests finish
dlm_proxy_ast_handler
dlm_do_local_bast
ocfs2_blocking_ast
ocfs2_generic_handle_bast
set OCFS2_LOCK_BLOCKED flag
dio_end_io
dio_bio_end_aio
dio_complete
ocfs2_dio_end_io
ocfs2_dio_end_io_write
ocfs2_inode_lock
__ocfs2_cluster_lock
ocfs2_wait_for_mask
-->waiting for OCFS2_LOCK_BLOCKED
flag to be cleared, that is waiting
for 'process 1' unlocking the inode lock
inode_dio_end
-->here dec the i_dio_count, but will never
be called, so a deadlock happened.

---
 fs/ocfs2/file.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

@@ -1151,6 +1151,13 @@ int ocfs2_setattr(struct dentry *dentry,
 		dquot_initialize(inode);
 	size_change = S_ISREG(inode->i_mode) && attr->ia_valid & ATTR_SIZE;
 	if (size_change) {
+		/*
+		 * Here we should wait dio to finish before inode lock
+		 * to avoid a deadlock between ocfs2_setattr() and
+		 * ocfs2_dio_end_io_write()
+		 */
+		inode_dio_wait(inode);
+
 		status = ocfs2_rw_lock(inode, 1);
 		if (status < 0) {
 			mlog_errno(status);

@@ -1170,8 +1177,6 @@ int ocfs2_setattr(struct dentry *dentry,
 		if (status)
 			goto bail_unlock;
 
-		inode_dio_wait(inode);
-
 		if (i_size_read(inode) >= attr->ia_size) {
 			if (ocfs2_should_order_data(inode)) {
 				status = ocfs2_begin_ordered_truncate(inode,

3.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Corey Minyard <[email protected]>

commit 392a17b10ec4320d3c0e96e2a23ebaad1123b989 upstream.

When I set the timeout to a specific value such as 500ms, the timeout event will not happen in time due to the overflow in function check_msg_timeout:
...

	ent->timeout -= timeout_period;
	if (ent->timeout > 0)
		return;

...

The type of timeout_period is long, but ent->timeout is unsigned long. This patch makes the type consistent.

---
 drivers/char/ipmi/ipmi_msghandler.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

@@ -4010,7 +4010,8 @@ smi_from_recv_msg(ipmi_smi_t intf, struc
 }
 
 static void check_msg_timeout(ipmi_smi_t intf, struct seq_table *ent,
-			      struct list_head *timeouts, long timeout_period,
+			      struct list_head *timeouts,
+			      unsigned long timeout_period,
 			      int slot, unsigned long *flags,
 			      unsigned int *waiting_msgs)
 {

@@ -4023,8 +4024,8 @@ static void check_msg_timeout(ipmi_smi_t
 	if (!ent->inuse)
 		return;
 
-	ent->timeout -= timeout_period;
-	if (ent->timeout > 0) {
+	if (timeout_period < ent->timeout) {
+		ent->timeout -= timeout_period;
 		(*waiting_msgs)++;
 		return;
 	}

@@ -4091,7 +4092,8 @@ static void check_msg_timeout(ipmi_smi_t
 	}
 }
 
-static unsigned int ipmi_timeout_handler(ipmi_smi_t intf, long timeout_period)
+static unsigned int ipmi_timeout_handler(ipmi_smi_t intf,
+					 unsigned long timeout_period)
 {
 	struct list_head     timeouts;
 	struct ipmi_recv_msg *msg, *msg2;

3.18-stable review patch. If anyone has any objections, please let me know.

------------------

From: Jan Harkes <[email protected]>

commit d337b66a4c52c7b04eec661d86c2ef6e168965a2 upstream.

When an application called fsync on a file in Coda a small request with just the file identifier was allocated, but the declared length was set to the size of union of all possible upcall requests.

This bug has been around for a very long time and is now caught by the extra checking in usercopy that was introduced in Linux-4.8.

The exposure happens when the Coda cache manager process reads the fsync upcall request at which point it is killed. As a result there is nobody servicing any further upcalls, trapping any processes that try to access the mounted Coda filesystem.

---
 fs/coda/upcall.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

@@ -446,8 +446,7 @@ int venus_fsync(struct super_block *sb,
 	UPARG(CODA_FSYNC);
 
 	inp->coda_fsync.VFid = *fid;
-	error = coda_upcall(coda_vcp(sb), sizeof(union inputArgs),
-			    &outsize, inp);
+	error = coda_upcall(coda_vcp(sb), insize, &outsize, inp);
 
 	CODA_FREE(inp, insize);
 	return error;

Are these "real"?

thanks,

greg k-h

Thanks for the manual verification on all of these.

greg k-h

Build results:

	total: 136 pass: 136 fail: 0
Qemu test results:
	total: 112 pass: 112 fail: 0

Details are available at http://kerneltests.org/builders.

Guenter