[PATCH 5.11 001/210] xfrm/compat: Cleanup WARN()s that can be user-triggered

[Greg Kroah-Hartman]

From: Dmitry Safonov 

commit ef19e111337f6c3dca7019a8bad5fbc6fb18d635 upstream.

Replace WARN_ONCE() that can be triggered from userspace with
pr_warn_once(). Those still give user a hint what's the issue.

I've left WARN()s that are not possible to trigger with current
code-base and that would mean that the code has issues:
- relying on current compat_msg_min[type] <= xfrm_msg_min[type]
- expected 4-byte padding size difference between
  compat_msg_min[type] and xfrm_msg_min[type]
- compat_policy[type].len <= xfrma_policy[type].len
(for every type)

Reported-by: syzbot+834ffd1afc7212eb8...@syzkaller.appspotmail.com
Fixes: 5f3eea6b7e8f ("xfrm/compat: Attach xfrm dumps to 64=>32 bit translator")
Cc: "David S. Miller" 
Cc: Eric Dumazet 
Cc: Herbert Xu 
Cc: Jakub Kicinski 
Cc: Steffen Klassert 
Cc: netdev@vger.kernel.org
Cc: sta...@vger.kernel.org
Signed-off-by: Dmitry Safonov 
Signed-off-by: Steffen Klassert 
Signed-off-by: Greg Kroah-Hartman 
---
 net/xfrm/xfrm_compat.c |   12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

--- a/net/xfrm/xfrm_compat.c
+++ b/net/xfrm/xfrm_compat.c
@@ -216,7 +216,7 @@ static struct nlmsghdr *xfrm_nlmsg_put_c
case XFRM_MSG_GETSADINFO:
case XFRM_MSG_GETSPDINFO:
default:
-   WARN_ONCE(1, "unsupported nlmsg_type %d", nlh_src->nlmsg_type);
+   pr_warn_once("unsupported nlmsg_type %d\n", 
nlh_src->nlmsg_type);
return ERR_PTR(-EOPNOTSUPP);
}
 
@@ -277,7 +277,7 @@ static int xfrm_xlate64_attr(struct sk_b
return xfrm_nla_cpy(dst, src, nla_len(src));
default:
BUILD_BUG_ON(XFRMA_MAX != XFRMA_IF_ID);
-   WARN_ONCE(1, "unsupported nla_type %d", src->nla_type);
+   pr_warn_once("unsupported nla_type %d\n", src->nla_type);
return -EOPNOTSUPP;
}
 }
@@ -315,8 +315,10 @@ static int xfrm_alloc_compat(struct sk_b
struct sk_buff *new = NULL;
int err;
 
-   if (WARN_ON_ONCE(type >= ARRAY_SIZE(xfrm_msg_min)))
+   if (type >= ARRAY_SIZE(xfrm_msg_min)) {
+   pr_warn_once("unsupported nlmsg_type %d\n", 
nlh_src->nlmsg_type);
return -EOPNOTSUPP;
+   }
 
if (skb_shinfo(skb)->frag_list == NULL) {
new = alloc_skb(skb->len + skb_tailroom(skb), GFP_ATOMIC);
@@ -378,6 +380,10 @@ static int xfrm_attr_cpy32(void *dst, si
struct nlmsghdr *nlmsg = dst;
struct nlattr *nla;
 
+   /* xfrm_user_rcv_msg_compat() relies on fact that 32-bit messages
+* have the same len or shorted than 64-bit ones.
+* 32-bit translation that is bigger than 64-bit original is unexpected.
+*/
if (WARN_ON_ONCE(copy_len > payload))
copy_len = payload;
 

[PATCH 5.10 001/188] xfrm/compat: Cleanup WARN()s that can be user-triggered

[Greg Kroah-Hartman]

From: Dmitry Safonov 

commit ef19e111337f6c3dca7019a8bad5fbc6fb18d635 upstream.

Replace WARN_ONCE() that can be triggered from userspace with
pr_warn_once(). Those still give user a hint what's the issue.

I've left WARN()s that are not possible to trigger with current
code-base and that would mean that the code has issues:
- relying on current compat_msg_min[type] <= xfrm_msg_min[type]
- expected 4-byte padding size difference between
  compat_msg_min[type] and xfrm_msg_min[type]
- compat_policy[type].len <= xfrma_policy[type].len
(for every type)

Reported-by: syzbot+834ffd1afc7212eb8...@syzkaller.appspotmail.com
Fixes: 5f3eea6b7e8f ("xfrm/compat: Attach xfrm dumps to 64=>32 bit translator")
Cc: "David S. Miller" 
Cc: Eric Dumazet 
Cc: Herbert Xu 
Cc: Jakub Kicinski 
Cc: Steffen Klassert 
Cc: netdev@vger.kernel.org
Cc: sta...@vger.kernel.org
Signed-off-by: Dmitry Safonov 
Signed-off-by: Steffen Klassert 
Signed-off-by: Greg Kroah-Hartman 
---
 net/xfrm/xfrm_compat.c |   12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

--- a/net/xfrm/xfrm_compat.c
+++ b/net/xfrm/xfrm_compat.c
@@ -216,7 +216,7 @@ static struct nlmsghdr *xfrm_nlmsg_put_c
case XFRM_MSG_GETSADINFO:
case XFRM_MSG_GETSPDINFO:
default:
-   WARN_ONCE(1, "unsupported nlmsg_type %d", nlh_src->nlmsg_type);
+   pr_warn_once("unsupported nlmsg_type %d\n", 
nlh_src->nlmsg_type);
return ERR_PTR(-EOPNOTSUPP);
}
 
@@ -277,7 +277,7 @@ static int xfrm_xlate64_attr(struct sk_b
return xfrm_nla_cpy(dst, src, nla_len(src));
default:
BUILD_BUG_ON(XFRMA_MAX != XFRMA_IF_ID);
-   WARN_ONCE(1, "unsupported nla_type %d", src->nla_type);
+   pr_warn_once("unsupported nla_type %d\n", src->nla_type);
return -EOPNOTSUPP;
}
 }
@@ -315,8 +315,10 @@ static int xfrm_alloc_compat(struct sk_b
struct sk_buff *new = NULL;
int err;
 
-   if (WARN_ON_ONCE(type >= ARRAY_SIZE(xfrm_msg_min)))
+   if (type >= ARRAY_SIZE(xfrm_msg_min)) {
+   pr_warn_once("unsupported nlmsg_type %d\n", 
nlh_src->nlmsg_type);
return -EOPNOTSUPP;
+   }
 
if (skb_shinfo(skb)->frag_list == NULL) {
new = alloc_skb(skb->len + skb_tailroom(skb), GFP_ATOMIC);
@@ -378,6 +380,10 @@ static int xfrm_attr_cpy32(void *dst, si
struct nlmsghdr *nlmsg = dst;
struct nlattr *nla;
 
+   /* xfrm_user_rcv_msg_compat() relies on fact that 32-bit messages
+* have the same len or shorted than 64-bit ones.
+* 32-bit translation that is bigger than 64-bit original is unexpected.
+*/
if (WARN_ON_ONCE(copy_len > payload))
copy_len = payload;
 

[PATCH 5.11 039/152] io_uring: call req_set_fail_links() on short send[msg]()/recv[msg]() with MSG_WAITALL

[Greg Kroah-Hartman]

From: Stefan Metzmacher 

[ Upstream commit 0031275d119efe16711cd93519b595e6f9b4b330 ]

Without that it's not safe to use them in a linked combination with
others.

Now combinations like IORING_OP_SENDMSG followed by IORING_OP_SPLICE
should be possible.

We already handle short reads and writes for the following opcodes:

- IORING_OP_READV
- IORING_OP_READ_FIXED
- IORING_OP_READ
- IORING_OP_WRITEV
- IORING_OP_WRITE_FIXED
- IORING_OP_WRITE
- IORING_OP_SPLICE
- IORING_OP_TEE

Now we have it for these as well:

- IORING_OP_SENDMSG
- IORING_OP_SEND
- IORING_OP_RECVMSG
- IORING_OP_RECV

For IORING_OP_RECVMSG we also check for the MSG_TRUNC and MSG_CTRUNC
flags in order to call req_set_fail_links().

There might be applications arround depending on the behavior
that even short send[msg]()/recv[msg]() retuns continue an
IOSQE_IO_LINK chain.

It's very unlikely that such applications pass in MSG_WAITALL,
which is only defined in 'man 2 recvmsg', but not in 'man 2 sendmsg'.

It's expected that the low level sock_sendmsg() call just ignores
MSG_WAITALL, as MSG_ZEROCOPY is also ignored without explicitly set
SO_ZEROCOPY.

We also expect the caller to know about the implicit truncation to
MAX_RW_COUNT, which we don't detect.

cc: netdev@vger.kernel.org
Link: 
https://lore.kernel.org/r/c4e1a4cc0d905314f4d5dc567e65a7b09621aab3.1615908477.git.me...@samba.org
Signed-off-by: Stefan Metzmacher 
Signed-off-by: Jens Axboe 
Signed-off-by: Sasha Levin 
---
 fs/io_uring.c | 24 
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index 26b4af9831da..2b0b9c3dda33 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -4628,6 +4628,7 @@ static int io_sendmsg(struct io_kiocb *req, bool 
force_nonblock,
struct io_async_msghdr iomsg, *kmsg;
struct socket *sock;
unsigned flags;
+   int min_ret = 0;
int ret;
 
sock = sock_from_file(req->file);
@@ -4654,6 +4655,9 @@ static int io_sendmsg(struct io_kiocb *req, bool 
force_nonblock,
else if (force_nonblock)
flags |= MSG_DONTWAIT;
 
+   if (flags & MSG_WAITALL)
+   min_ret = iov_iter_count(&kmsg->msg.msg_iter);
+
ret = __sys_sendmsg_sock(sock, &kmsg->msg, flags);
if (force_nonblock && ret == -EAGAIN)
return io_setup_async_msg(req, kmsg);
@@ -4663,7 +4667,7 @@ static int io_sendmsg(struct io_kiocb *req, bool 
force_nonblock,
if (kmsg->iov != kmsg->fast_iov)
kfree(kmsg->iov);
req->flags &= ~REQ_F_NEED_CLEANUP;
-   if (ret < 0)
+   if (ret < min_ret)
req_set_fail_links(req);
__io_req_complete(req, ret, 0, cs);
return 0;
@@ -4677,6 +4681,7 @@ static int io_send(struct io_kiocb *req, bool 
force_nonblock,
struct iovec iov;
struct socket *sock;
unsigned flags;
+   int min_ret = 0;
int ret;
 
sock = sock_from_file(req->file);
@@ -4698,6 +4703,9 @@ static int io_send(struct io_kiocb *req, bool 
force_nonblock,
else if (force_nonblock)
flags |= MSG_DONTWAIT;
 
+   if (flags & MSG_WAITALL)
+   min_ret = iov_iter_count(&msg.msg_iter);
+
msg.msg_flags = flags;
ret = sock_sendmsg(sock, &msg);
if (force_nonblock && ret == -EAGAIN)
@@ -4705,7 +4713,7 @@ static int io_send(struct io_kiocb *req, bool 
force_nonblock,
if (ret == -ERESTARTSYS)
ret = -EINTR;
 
-   if (ret < 0)
+   if (ret < min_ret)
req_set_fail_links(req);
__io_req_complete(req, ret, 0, cs);
return 0;
@@ -4857,6 +4865,7 @@ static int io_recvmsg(struct io_kiocb *req, bool 
force_nonblock,
struct socket *sock;
struct io_buffer *kbuf;
unsigned flags;
+   int min_ret = 0;
int ret, cflags = 0;
 
sock = sock_from_file(req->file);
@@ -4892,6 +4901,9 @@ static int io_recvmsg(struct io_kiocb *req, bool 
force_nonblock,
else if (force_nonblock)
flags |= MSG_DONTWAIT;
 
+   if (flags & MSG_WAITALL)
+   min_ret = iov_iter_count(&kmsg->msg.msg_iter);
+
ret = __sys_recvmsg_sock(sock, &kmsg->msg, req->sr_msg.umsg,
kmsg->uaddr, flags);
if (force_nonblock && ret == -EAGAIN)
@@ -4904,7 +4916,7 @@ static int io_recvmsg(struct io_kiocb *req, bool 
force_nonblock,
if (kmsg->iov != kmsg->fast_iov)
kfree(kmsg->iov);
req->flags &= ~REQ_F_NEED_CLEANUP;
-   if (ret < 0)
+   if (ret < min_ret || ((flags & MSG_WAITALL) && (kmsg->msg.msg_flags & 
(MSG_TRUNC | MSG_CTRUNC
req_set_fail_links(req);
__io_req_complete(req, ret, cflags, cs);
return 0;
@@ -4920,6 +4932,7 @@ static int io_recv(struct io_kiocb *req, bool 
force_nonblock,
struct socket *sock;
struct iovec iov;
unsigned flags;
+   int min_ret = 0;
int ret,

[PATCH 5.10 036/126] io_uring: call req_set_fail_links() on short send[msg]()/recv[msg]() with MSG_WAITALL

[Greg Kroah-Hartman]

From: Stefan Metzmacher 

[ Upstream commit 0031275d119efe16711cd93519b595e6f9b4b330 ]

Without that it's not safe to use them in a linked combination with
others.

Now combinations like IORING_OP_SENDMSG followed by IORING_OP_SPLICE
should be possible.

We already handle short reads and writes for the following opcodes:

- IORING_OP_READV
- IORING_OP_READ_FIXED
- IORING_OP_READ
- IORING_OP_WRITEV
- IORING_OP_WRITE_FIXED
- IORING_OP_WRITE
- IORING_OP_SPLICE
- IORING_OP_TEE

Now we have it for these as well:

- IORING_OP_SENDMSG
- IORING_OP_SEND
- IORING_OP_RECVMSG
- IORING_OP_RECV

For IORING_OP_RECVMSG we also check for the MSG_TRUNC and MSG_CTRUNC
flags in order to call req_set_fail_links().

There might be applications arround depending on the behavior
that even short send[msg]()/recv[msg]() retuns continue an
IOSQE_IO_LINK chain.

It's very unlikely that such applications pass in MSG_WAITALL,
which is only defined in 'man 2 recvmsg', but not in 'man 2 sendmsg'.

It's expected that the low level sock_sendmsg() call just ignores
MSG_WAITALL, as MSG_ZEROCOPY is also ignored without explicitly set
SO_ZEROCOPY.

We also expect the caller to know about the implicit truncation to
MAX_RW_COUNT, which we don't detect.

cc: netdev@vger.kernel.org
Link: 
https://lore.kernel.org/r/c4e1a4cc0d905314f4d5dc567e65a7b09621aab3.1615908477.git.me...@samba.org
Signed-off-by: Stefan Metzmacher 
Signed-off-by: Jens Axboe 
Signed-off-by: Sasha Levin 
---
 fs/io_uring.c | 24 
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/fs/io_uring.c b/fs/io_uring.c
index fe2dfdab0acd..4ccf99cb8cdc 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -4401,6 +4401,7 @@ static int io_sendmsg(struct io_kiocb *req, bool 
force_nonblock,
struct io_async_msghdr iomsg, *kmsg;
struct socket *sock;
unsigned flags;
+   int min_ret = 0;
int ret;
 
sock = sock_from_file(req->file, &ret);
@@ -4427,6 +4428,9 @@ static int io_sendmsg(struct io_kiocb *req, bool 
force_nonblock,
else if (force_nonblock)
flags |= MSG_DONTWAIT;
 
+   if (flags & MSG_WAITALL)
+   min_ret = iov_iter_count(&kmsg->msg.msg_iter);
+
ret = __sys_sendmsg_sock(sock, &kmsg->msg, flags);
if (force_nonblock && ret == -EAGAIN)
return io_setup_async_msg(req, kmsg);
@@ -4436,7 +4440,7 @@ static int io_sendmsg(struct io_kiocb *req, bool 
force_nonblock,
if (kmsg->iov != kmsg->fast_iov)
kfree(kmsg->iov);
req->flags &= ~REQ_F_NEED_CLEANUP;
-   if (ret < 0)
+   if (ret < min_ret)
req_set_fail_links(req);
__io_req_complete(req, ret, 0, cs);
return 0;
@@ -4450,6 +4454,7 @@ static int io_send(struct io_kiocb *req, bool 
force_nonblock,
struct iovec iov;
struct socket *sock;
unsigned flags;
+   int min_ret = 0;
int ret;
 
sock = sock_from_file(req->file, &ret);
@@ -4471,6 +4476,9 @@ static int io_send(struct io_kiocb *req, bool 
force_nonblock,
else if (force_nonblock)
flags |= MSG_DONTWAIT;
 
+   if (flags & MSG_WAITALL)
+   min_ret = iov_iter_count(&msg.msg_iter);
+
msg.msg_flags = flags;
ret = sock_sendmsg(sock, &msg);
if (force_nonblock && ret == -EAGAIN)
@@ -4478,7 +4486,7 @@ static int io_send(struct io_kiocb *req, bool 
force_nonblock,
if (ret == -ERESTARTSYS)
ret = -EINTR;
 
-   if (ret < 0)
+   if (ret < min_ret)
req_set_fail_links(req);
__io_req_complete(req, ret, 0, cs);
return 0;
@@ -4630,6 +4638,7 @@ static int io_recvmsg(struct io_kiocb *req, bool 
force_nonblock,
struct socket *sock;
struct io_buffer *kbuf;
unsigned flags;
+   int min_ret = 0;
int ret, cflags = 0;
 
sock = sock_from_file(req->file, &ret);
@@ -4665,6 +4674,9 @@ static int io_recvmsg(struct io_kiocb *req, bool 
force_nonblock,
else if (force_nonblock)
flags |= MSG_DONTWAIT;
 
+   if (flags & MSG_WAITALL)
+   min_ret = iov_iter_count(&kmsg->msg.msg_iter);
+
ret = __sys_recvmsg_sock(sock, &kmsg->msg, req->sr_msg.umsg,
kmsg->uaddr, flags);
if (force_nonblock && ret == -EAGAIN)
@@ -4677,7 +4689,7 @@ static int io_recvmsg(struct io_kiocb *req, bool 
force_nonblock,
if (kmsg->iov != kmsg->fast_iov)
kfree(kmsg->iov);
req->flags &= ~REQ_F_NEED_CLEANUP;
-   if (ret < 0)
+   if (ret < min_ret || ((flags & MSG_WAITALL) && (kmsg->msg.msg_flags & 
(MSG_TRUNC | MSG_CTRUNC
req_set_fail_links(req);
__io_req_complete(req, ret, cflags, cs);
return 0;
@@ -4693,6 +4705,7 @@ static int io_recv(struct io_kiocb *req, bool 
force_nonblock,
struct socket *sock;
struct iovec iov;
unsigned flags;
+   int min_ret = 0

Re: [PATCH mlx5-next v7 0/4] Dynamically assign MSI-X vectors count

[Greg Kroah-Hartman]

On Wed, Mar 31, 2021 at 09:19:29AM -0300, Jason Gunthorpe wrote:
> On Wed, Mar 31, 2021 at 08:38:39AM +0200, Greg Kroah-Hartman wrote:
> > On Tue, Mar 30, 2021 at 07:43:41PM -0300, Jason Gunthorpe wrote:
> > > > With :01:00.0/sriov/BB:DD.F/vf_msix_count, sriov/ will contain
> > > > 1 file and 1K subdirectories.
> > > 
> > > The smallest directory sizes is with the current patch since it
> > > re-uses the existing VF directory. Do we care about directory size at
> > > the sysfs level?
> > 
> > No, that should not matter.
> > 
> > The "issue" here is that you "broke" the device chain here by adding a
> > random kobject to the directory tree: "BB:DD.F"
> > 
> > Again, devices are allowed to have attributes associated with it to be
> > _ONE_ subdirectory level deep.
> > 
> > So, to use your path above, this is allowed:
> > :01:00.0/sriov/vf_msix_count
> > 
> > as these are sriov attributes for the :01:00.0 device, but this is
> > not:
> > :01:00.0/sriov/BB:DD.F/vf_msix_count
> > as you "threw" a random kobject called BB:DD.F into the middle.
> >
> > If you want to have "BB:DD.F" in there, then it needs to be a real
> > struct device and _THEN_ it needs to point its parent to ":01:00.0",
> > another struct device, as "sriov" is NOT ANYTHING in the heirachy here
> > at all.
> 
> It isn't a struct device object at all though, it just organizing
> attributes.

That's the point, it really is not.  You are forced to create a real
object for that subdirectory, and by doing so, you are "breaking" the
driver/device model.  As is evident by userspace not knowing what is
going on here.

> > Does that help?  The rules are:
> > - Once you use a 'struct device', all subdirs below that device
> >   are either an attribute group for that device or a child
> >   device.
> > - A struct device can NOT have an attribute group as a parent,
> >   it can ONLY have another struct device as a parent.
> > 
> > If you break those rules, the kernel has the ability to get really
> > confused unless you are very careful, and userspace will be totally lost
> > as you can not do anything special there.
> 
> The kernel gets confused?

Putting a kobject as a child of a struct device can easily cause
confusion as that is NOT what you should be doing.  Especially if you
then try to add a device to be a child of that kobject.  Now the kernel
core can not walk all devices in the correct order and lots of other
problems that you are lucky you do not hit.

> I'm not sure I understand why userspace gets confused. I can guess
> udev has some issue, but everything else seems OK, it is just a path.

No, it is not a "path".

Again, here are the driver/device model rules:
- once you have a "struct device", only "struct device" can be
  children.
- You are allowed to place attributes in subdirectories if you
  want to make things cleaner.  Don't make me doubt giving that
  type of permission to people by trying to abuse it by going
  "lower" than one level.
- If you have to represent something dynamic below a struct
  device that is not an attribute, make it a real struct device.

And userspace "gets confused" because it thinks it can properly walk the
tree and get a record of all devices in the system.  When you put a
kobject in there just to make a new subdirectory, you break the
notification model and everything else.

Again, do not do that.

> > > > I'm dense and don't fully understand Greg's subdirectory comment.
> > > 
> > > I also don't know udev well enough. I've certainly seen drivers
> > > creating extra subdirectories using kobjects.
> > 
> > And those drivers are broken.  Please point them out to me and I will be
> > glad to go fix them.  Or tell their authors why they are broken :)
> 
> How do you fix them? It is uAPI at this point so we can't change the
> directory names. Can't make them struct devices (userspace would get
> confused if we add *more* sysfs files)

How would userspace get confused?  If anything it would suddenly "wake
up" and see these attributes properly.

> Grep for kobject_init_and_add() under drivers/ and I think you get a
> pretty good overview of the places.

Yes, lots of places where people are abusing things and it should not be
done.  Do not add to the mess by knowingly adding broken code please.

> Since it seems like kind of a big problem can we make th

Re: [PATCH mlx5-next v7 0/4] Dynamically assign MSI-X vectors count

[Greg Kroah-Hartman]

On Tue, Mar 30, 2021 at 07:43:41PM -0300, Jason Gunthorpe wrote:
> > With :01:00.0/sriov/BB:DD.F/vf_msix_count, sriov/ will contain
> > 1 file and 1K subdirectories.
> 
> The smallest directory sizes is with the current patch since it
> re-uses the existing VF directory. Do we care about directory size at
> the sysfs level?

No, that should not matter.

The "issue" here is that you "broke" the device chain here by adding a
random kobject to the directory tree: "BB:DD.F"

Again, devices are allowed to have attributes associated with it to be
_ONE_ subdirectory level deep.

So, to use your path above, this is allowed:
:01:00.0/sriov/vf_msix_count

as these are sriov attributes for the :01:00.0 device, but this is
not:
:01:00.0/sriov/BB:DD.F/vf_msix_count
as you "threw" a random kobject called BB:DD.F into the middle.

If you want to have "BB:DD.F" in there, then it needs to be a real
struct device and _THEN_ it needs to point its parent to ":01:00.0",
another struct device, as "sriov" is NOT ANYTHING in the heirachy here
at all.

Does that help?  The rules are:
- Once you use a 'struct device', all subdirs below that device
  are either an attribute group for that device or a child
  device.
- A struct device can NOT have an attribute group as a parent,
  it can ONLY have another struct device as a parent.

If you break those rules, the kernel has the ability to get really
confused unless you are very careful, and userspace will be totally lost
as you can not do anything special there.

> > I'm dense and don't fully understand Greg's subdirectory comment.
> 
> I also don't know udev well enough. I've certainly seen drivers
> creating extra subdirectories using kobjects.

And those drivers are broken.  Please point them out to me and I will be
glad to go fix them.  Or tell their authors why they are broken :)

> > But it doesn't seem like that level of control would be in a udev rule
> > anyway.  A PF udev rule might *start* a program to manage MSI-X
> > vectors, but such a program should be able to deal with whatever
> > directory structure we want.
> 
> Yes, I can't really see this being used from udev either. 

It doesn't matter if you think it could be used, it _will_ be used as
you are exposing this stuff to userspace.

> I assume there is also the usual race about triggering the uevent
> before the subdirectories are created, but we have the
> dev_set_uevent_suppress() thing now for that..

Unless you are "pci bus code" you shouldn't be using that :)

thanks,

greg k-h

Re: [PATCH net-next] qrtr: move to staging

[Greg Kroah-Hartman]

On Mon, Mar 29, 2021 at 04:22:36PM +0530, Manivannan Sadhasivam wrote:
> Hi Greg,
> 
> On Mon, Mar 29, 2021 at 11:47:12AM +0200, Loic Poulain wrote:
> > Hi Greg,
> > 
> > On Sun, 28 Mar 2021 at 14:28, Greg Kroah-Hartman 
> > 
> > wrote:
> > 
> > > There does not seem to be any developers willing to maintain the
> > > net/qrtr/ code, so move it to drivers/staging/ so that it can be removed
> > > from the kernel tree entirely in a few kernel releases if no one steps
> > > up to maintain it.
> > >
> > > Reported-by: Matthew Wilcox 
> > > Cc: Du Cheng 
> > > Signed-off-by: Greg Kroah-Hartman 
> > >
> > 
> > As far as I know, QRTR/IPCR is still commonly used with Qualcomm-based
> > platforms for accessing various components of the SoC.
> > CCing Bjorn and Mani, In case they are interested in taking maintenance of
> > that.
> > 
> 
> As Loic said, QRTR is an integral component used in various Qualcomm based
> upstream supported products like ChromeOS, newer WLAN chipsets (QCA6390) 
> etc...
> 
> It is unfortunate that no one stepped up so far to maintain it. After
> having an internal discussion, I decided to pitch in as a maintainer. I'll
> send the MAINTAINERS change to netdev list now.

Great, can you also fix up the reported problems with the codebase that
resulted in this "ask for removal"?

thanks,

greg k-h

Re: [PATCH net-next] qrtr: move to staging

[Greg Kroah-Hartman]

On Mon, Mar 29, 2021 at 09:30:25AM +0300, Leon Romanovsky wrote:
> On Mon, Mar 29, 2021 at 07:30:08AM +0200, Greg Kroah-Hartman wrote:
> > On Mon, Mar 29, 2021 at 08:17:06AM +0300, Leon Romanovsky wrote:
> > > On Sun, Mar 28, 2021 at 02:26:21PM +0200, Greg Kroah-Hartman wrote:
> > > > There does not seem to be any developers willing to maintain the
> > > > net/qrtr/ code, so move it to drivers/staging/ so that it can be removed
> > > > from the kernel tree entirely in a few kernel releases if no one steps
> > > > up to maintain it.
> > > > 
> > > > Reported-by: Matthew Wilcox 
> > > > Cc: Du Cheng 
> > > > Signed-off-by: Greg Kroah-Hartman 
> > > > ---
> > > 
> > > Greg,
> > > 
> > > Why don't you simply delete it like other code that is not maintained?
> > 
> > "normally" we have been giving code a chance by having it live in
> > drivers/staging/ for a bit before removing it to allow anyone that
> > actually cares about the codebase to notice it before removing it.
> 
> I don't know about netdev view on this, but for the RDMA code, the code
> in staging means _not_exist_. We took this decision after/during Lustre
> fiasco. 

That's fine, each subsystem can set it's own rules for staging code.
For networking stuff, the flow-out-through-staging has been happening
for some time now.

Lustre was a different beast, that was an attempt to get code into the
kernel, not out.

thanks,

greg k-h

Re: [PATCH net-next] qrtr: move to staging

[Greg Kroah-Hartman]

On Mon, Mar 29, 2021 at 08:17:06AM +0300, Leon Romanovsky wrote:
> On Sun, Mar 28, 2021 at 02:26:21PM +0200, Greg Kroah-Hartman wrote:
> > There does not seem to be any developers willing to maintain the
> > net/qrtr/ code, so move it to drivers/staging/ so that it can be removed
> > from the kernel tree entirely in a few kernel releases if no one steps
> > up to maintain it.
> > 
> > Reported-by: Matthew Wilcox 
> > Cc: Du Cheng 
> > Signed-off-by: Greg Kroah-Hartman 
> > ---
> 
> Greg,
> 
> Why don't you simply delete it like other code that is not maintained?

"normally" we have been giving code a chance by having it live in
drivers/staging/ for a bit before removing it to allow anyone that
actually cares about the codebase to notice it before removing it.

We've done this for many drivers and code-chunks over the years, wimax
was one recent example.

Just trying to be nice :)

thanks,

greg k-h

[PATCH net-next] qrtr: move to staging

[Greg Kroah-Hartman]

There does not seem to be any developers willing to maintain the
net/qrtr/ code, so move it to drivers/staging/ so that it can be removed
from the kernel tree entirely in a few kernel releases if no one steps
up to maintain it.

Reported-by: Matthew Wilcox 
Cc: Du Cheng 
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/staging/Kconfig| 2 ++
 drivers/staging/Makefile   | 1 +
 {net => drivers/staging}/qrtr/Kconfig  | 1 +
 {net => drivers/staging}/qrtr/Makefile | 0
 {net => drivers/staging}/qrtr/mhi.c| 0
 {net => drivers/staging}/qrtr/ns.c | 0
 {net => drivers/staging}/qrtr/qrtr.c   | 0
 {net => drivers/staging}/qrtr/qrtr.h   | 0
 {net => drivers/staging}/qrtr/smd.c| 0
 {net => drivers/staging}/qrtr/tun.c| 0
 net/Kconfig| 1 -
 net/Makefile   | 1 -
 12 files changed, 4 insertions(+), 2 deletions(-)
 rename {net => drivers/staging}/qrtr/Kconfig (98%)
 rename {net => drivers/staging}/qrtr/Makefile (100%)
 rename {net => drivers/staging}/qrtr/mhi.c (100%)
 rename {net => drivers/staging}/qrtr/ns.c (100%)
 rename {net => drivers/staging}/qrtr/qrtr.c (100%)
 rename {net => drivers/staging}/qrtr/qrtr.h (100%)
 rename {net => drivers/staging}/qrtr/smd.c (100%)
 rename {net => drivers/staging}/qrtr/tun.c (100%)

diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 6e798229fe25..43ab47e7861c 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -112,4 +112,6 @@ source "drivers/staging/wfx/Kconfig"
 
 source "drivers/staging/hikey9xx/Kconfig"
 
+source "drivers/staging/qrtr/Kconfig"
+
 endif # STAGING
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 8d4d9812ecdf..0b7ae2a72954 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -45,4 +45,5 @@ obj-$(CONFIG_KPC2000) += kpc2000/
 obj-$(CONFIG_QLGE) += qlge/
 obj-$(CONFIG_WIMAX)+= wimax/
 obj-$(CONFIG_WFX)  += wfx/
+obj-$(CONFIG_QRTR) += qrtr/
 obj-y  += hikey9xx/
diff --git a/net/qrtr/Kconfig b/drivers/staging/qrtr/Kconfig
similarity index 98%
rename from net/qrtr/Kconfig
rename to drivers/staging/qrtr/Kconfig
index b4020b84760f..e91825f01307 100644
--- a/net/qrtr/Kconfig
+++ b/drivers/staging/qrtr/Kconfig
@@ -4,6 +4,7 @@
 
 config QRTR
tristate "Qualcomm IPC Router support"
+   depends on NET
help
  Say Y if you intend to use Qualcomm IPC router protocol.  The
  protocol is used to communicate with services provided by other
diff --git a/net/qrtr/Makefile b/drivers/staging/qrtr/Makefile
similarity index 100%
rename from net/qrtr/Makefile
rename to drivers/staging/qrtr/Makefile
diff --git a/net/qrtr/mhi.c b/drivers/staging/qrtr/mhi.c
similarity index 100%
rename from net/qrtr/mhi.c
rename to drivers/staging/qrtr/mhi.c
diff --git a/net/qrtr/ns.c b/drivers/staging/qrtr/ns.c
similarity index 100%
rename from net/qrtr/ns.c
rename to drivers/staging/qrtr/ns.c
diff --git a/net/qrtr/qrtr.c b/drivers/staging/qrtr/qrtr.c
similarity index 100%
rename from net/qrtr/qrtr.c
rename to drivers/staging/qrtr/qrtr.c
diff --git a/net/qrtr/qrtr.h b/drivers/staging/qrtr/qrtr.h
similarity index 100%
rename from net/qrtr/qrtr.h
rename to drivers/staging/qrtr/qrtr.h
diff --git a/net/qrtr/smd.c b/drivers/staging/qrtr/smd.c
similarity index 100%
rename from net/qrtr/smd.c
rename to drivers/staging/qrtr/smd.c
diff --git a/net/qrtr/tun.c b/drivers/staging/qrtr/tun.c
similarity index 100%
rename from net/qrtr/tun.c
rename to drivers/staging/qrtr/tun.c
diff --git a/net/Kconfig b/net/Kconfig
index 9c456acc379e..09f14caf3f45 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -242,7 +242,6 @@ source "net/nsh/Kconfig"
 source "net/hsr/Kconfig"
 source "net/switchdev/Kconfig"
 source "net/l3mdev/Kconfig"
-source "net/qrtr/Kconfig"
 source "net/ncsi/Kconfig"
 
 config PCPU_DEV_REFCNT
diff --git a/net/Makefile b/net/Makefile
index 9ca9572188fe..5d57e972a33b 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -74,7 +74,6 @@ obj-$(CONFIG_NET_NSH) += nsh/
 obj-$(CONFIG_HSR)  += hsr/
 obj-$(CONFIG_NET_SWITCHDEV)+= switchdev/
 obj-$(CONFIG_NET_L3_MASTER_DEV)+= l3mdev/
-obj-$(CONFIG_QRTR) += qrtr/
 obj-$(CONFIG_NET_NCSI) += ncsi/
 obj-$(CONFIG_XDP_SOCKETS)  += xdp/
 obj-$(CONFIG_MPTCP)+= mptcp/
-- 
2.31.1

Re: [PATCH v2] net:qrtr: fix atomic idr allocation in qrtr_port_assign()

[Greg Kroah-Hartman]

On Sat, Mar 27, 2021 at 03:51:10PM +, Matthew Wilcox wrote:
> On Sat, Mar 27, 2021 at 03:31:18PM +0100, Greg Kroah-Hartman wrote:
> > On Sat, Mar 27, 2021 at 10:25:20PM +0800, Du Cheng wrote:
> > > On Sat, Mar 27, 2021 at 03:12:14PM +0100, Greg Kroah-Hartman wrote:
> > > > Adding the xarray maintainer...
> > > > 
> > > > On Sat, Mar 27, 2021 at 10:07:02PM +0800, Du Cheng wrote:
> > > > > add idr_preload() and idr_preload_end() around 
> > > > > idr_alloc_u32(GFP_ATOMIC)
> > > > > due to internal use of per_cpu variables, which requires preemption
> > > > > disabling/enabling.
> > > > > 
> > > > > reported as "BUG: "using smp_processor_id() in preemptible" by 
> > > > > syzkaller
> > > > > 
> > > > > Reported-by: syzbot+3eec59e770685e3dc...@syzkaller.appspotmail.com
> > > > > Signed-off-by: Du Cheng 
> > > > > ---
> > > > > changelog
> > > > > v1: change to GFP_KERNEL for idr_alloc_u32() but might sleep
> > > > > v2: revert to GFP_ATOMIC but add preemption disable/enable protection
> > > > > 
> > > > >  net/qrtr/qrtr.c | 6 ++
> > > > >  1 file changed, 6 insertions(+)
> > > > > 
> > > > > diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
> > > > > index edb6ac17ceca..6361f169490e 100644
> > > > > --- a/net/qrtr/qrtr.c
> > > > > +++ b/net/qrtr/qrtr.c
> > > > > @@ -722,17 +722,23 @@ static int qrtr_port_assign(struct qrtr_sock 
> > > > > *ipc, int *port)
> > > > >   mutex_lock(&qrtr_port_lock);
> > > > >   if (!*port) {
> > > > >   min_port = QRTR_MIN_EPH_SOCKET;
> > > > > + idr_preload(GFP_ATOMIC);
> > > > >   rc = idr_alloc_u32(&qrtr_ports, ipc, &min_port, 
> > > > > QRTR_MAX_EPH_SOCKET, GFP_ATOMIC);
> > > > > + idr_preload_end();
> > > > 
> > > > This seems "odd" to me.  We are asking idr_alloc_u32() to abide by
> > > > GFP_ATOMIC, so why do we need to "preload" it with the same type of
> > > > allocation?
> > > > 
> > > > Is there something in the idr/radix/xarray code that can't really handle
> > > > GFP_ATOMIC during a "normal" idr allocation that is causing this warning
> > > > to be hit?  Why is this change the "correct" one?
> > > > 
> > > > thanks,
> > > > 
> > > > greg k-h
> > > 
> > > 
> > > >From the comment above idr_preload() in lib/radix-tree.c:1460
> > > /**
> > >  * idr_preload - preload for idr_alloc()
> > >  * @gfp_mask: allocation mask to use for preloading
> > >  *
> > >  * Preallocate memory to use for the next call to idr_alloc().  This 
> > > function
> > >  * returns with preemption disabled.  It will be enabled by 
> > > idr_preload_end().
> > >  */
> > > 
> > > idr_alloc is a very simple wrapper around idr_alloc_u32().
> > > 
> > > On top of radix_tree_node_alloc() which is called by idr_alloc_u32(), 
> > > there is
> > > this comment at line 244 in the same radix-tree.c
> > > /*
> > >  * This assumes that the caller has performed appropriate preallocation, 
> > > and
> > >  * that the caller has pinned this thread of control to the current CPU.
> > >  */
> > > 
> > > Therefore the preload/preload_end are necessary, or at least should have
> > > preemption disabled
> > 
> > Ah, so it's disabling preemption that is the key here.  Still odd, why
> > is GFP_ATOMIC not sufficient in a normal idr_alloc() call to keep things
> > from doing stuff like this?  Feels like a lot of "internal knowledge" is
> > needed here to use this api properly...
> > 
> > Matthew, is the above change really correct?
> 
> No.
> 
> https://lore.kernel.org/netdev/20200605112922.gb19...@bombadil.infradead.org/
> https://lore.kernel.org/netdev/20200605120037.17427-1-wi...@infradead.org/
> https://lore.kernel.org/netdev/20200914192655.gw6...@casper.infradead.org/
> 

Ok, it looks like this code is just abandonded, should we remove it
entirely as no one wants to maintain it?

thanks,

greg k-h

Re: [PATCH v2] net:qrtr: fix atomic idr allocation in qrtr_port_assign()

[Greg Kroah-Hartman]

On Sat, Mar 27, 2021 at 10:25:20PM +0800, Du Cheng wrote:
> On Sat, Mar 27, 2021 at 03:12:14PM +0100, Greg Kroah-Hartman wrote:
> > Adding the xarray maintainer...
> > 
> > On Sat, Mar 27, 2021 at 10:07:02PM +0800, Du Cheng wrote:
> > > add idr_preload() and idr_preload_end() around idr_alloc_u32(GFP_ATOMIC)
> > > due to internal use of per_cpu variables, which requires preemption
> > > disabling/enabling.
> > > 
> > > reported as "BUG: "using smp_processor_id() in preemptible" by syzkaller
> > > 
> > > Reported-by: syzbot+3eec59e770685e3dc...@syzkaller.appspotmail.com
> > > Signed-off-by: Du Cheng 
> > > ---
> > > changelog
> > > v1: change to GFP_KERNEL for idr_alloc_u32() but might sleep
> > > v2: revert to GFP_ATOMIC but add preemption disable/enable protection
> > > 
> > >  net/qrtr/qrtr.c | 6 ++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
> > > index edb6ac17ceca..6361f169490e 100644
> > > --- a/net/qrtr/qrtr.c
> > > +++ b/net/qrtr/qrtr.c
> > > @@ -722,17 +722,23 @@ static int qrtr_port_assign(struct qrtr_sock *ipc, 
> > > int *port)
> > >   mutex_lock(&qrtr_port_lock);
> > >   if (!*port) {
> > >   min_port = QRTR_MIN_EPH_SOCKET;
> > > + idr_preload(GFP_ATOMIC);
> > >   rc = idr_alloc_u32(&qrtr_ports, ipc, &min_port, 
> > > QRTR_MAX_EPH_SOCKET, GFP_ATOMIC);
> > > + idr_preload_end();
> > 
> > This seems "odd" to me.  We are asking idr_alloc_u32() to abide by
> > GFP_ATOMIC, so why do we need to "preload" it with the same type of
> > allocation?
> > 
> > Is there something in the idr/radix/xarray code that can't really handle
> > GFP_ATOMIC during a "normal" idr allocation that is causing this warning
> > to be hit?  Why is this change the "correct" one?
> > 
> > thanks,
> > 
> > greg k-h
> 
> 
> >From the comment above idr_preload() in lib/radix-tree.c:1460
> /**
>  * idr_preload - preload for idr_alloc()
>  * @gfp_mask: allocation mask to use for preloading
>  *
>  * Preallocate memory to use for the next call to idr_alloc().  This function
>  * returns with preemption disabled.  It will be enabled by idr_preload_end().
>  */
> 
> idr_alloc is a very simple wrapper around idr_alloc_u32().
> 
> On top of radix_tree_node_alloc() which is called by idr_alloc_u32(), there is
> this comment at line 244 in the same radix-tree.c
> /*
>  * This assumes that the caller has performed appropriate preallocation, and
>  * that the caller has pinned this thread of control to the current CPU.
>  */
> 
> Therefore the preload/preload_end are necessary, or at least should have
> preemption disabled

Ah, so it's disabling preemption that is the key here.  Still odd, why
is GFP_ATOMIC not sufficient in a normal idr_alloc() call to keep things
from doing stuff like this?  Feels like a lot of "internal knowledge" is
needed here to use this api properly...

Matthew, is the above change really correct?

thanks,

greg k-h

Re: [PATCH v2] net:qrtr: fix atomic idr allocation in qrtr_port_assign()

[Greg Kroah-Hartman]

Adding the xarray maintainer...

On Sat, Mar 27, 2021 at 10:07:02PM +0800, Du Cheng wrote:
> add idr_preload() and idr_preload_end() around idr_alloc_u32(GFP_ATOMIC)
> due to internal use of per_cpu variables, which requires preemption
> disabling/enabling.
> 
> reported as "BUG: "using smp_processor_id() in preemptible" by syzkaller
> 
> Reported-by: syzbot+3eec59e770685e3dc...@syzkaller.appspotmail.com
> Signed-off-by: Du Cheng 
> ---
> changelog
> v1: change to GFP_KERNEL for idr_alloc_u32() but might sleep
> v2: revert to GFP_ATOMIC but add preemption disable/enable protection
> 
>  net/qrtr/qrtr.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
> index edb6ac17ceca..6361f169490e 100644
> --- a/net/qrtr/qrtr.c
> +++ b/net/qrtr/qrtr.c
> @@ -722,17 +722,23 @@ static int qrtr_port_assign(struct qrtr_sock *ipc, int 
> *port)
>   mutex_lock(&qrtr_port_lock);
>   if (!*port) {
>   min_port = QRTR_MIN_EPH_SOCKET;
> + idr_preload(GFP_ATOMIC);
>   rc = idr_alloc_u32(&qrtr_ports, ipc, &min_port, 
> QRTR_MAX_EPH_SOCKET, GFP_ATOMIC);
> + idr_preload_end();

This seems "odd" to me.  We are asking idr_alloc_u32() to abide by
GFP_ATOMIC, so why do we need to "preload" it with the same type of
allocation?

Is there something in the idr/radix/xarray code that can't really handle
GFP_ATOMIC during a "normal" idr allocation that is causing this warning
to be hit?  Why is this change the "correct" one?

thanks,

greg k-h

Re: [PATCH mlx5-next v7 0/4] Dynamically assign MSI-X vectors count

[Greg Kroah-Hartman]

On Fri, Mar 26, 2021 at 02:36:31PM -0500, Bjorn Helgaas wrote:
> On Fri, Mar 26, 2021 at 11:50:44AM -0700, Alexander Duyck wrote:
> 
> > I almost wonder if it wouldn't make sense to just partition this up to
> > handle flexible resources in the future. Maybe something like having
> > the directory setup such that you have "sriov_resources/msix/" and
> > then you could have individual files with one for the total and the
> > rest with the VF BDF naming scheme. Then if we have to, we could add
> > other subdirectories in the future to handle things like queues in the
> > future.
> 
> Subdirectories would be nice, but Greg KH said earlier in a different
> context that there's an issue with them [1].  He went on to say tools
> like udev would miss uevents for the subdirs [2].
> 
> I don't know whether that's really a problem in this case -- it
> doesn't seem like we would care about uevents for files that do MSI-X
> vector assignment.
> 
> [1] https://lore.kernel.org/linux-pci/20191121211017.ga854...@kroah.com/
> [2] https://lore.kernel.org/linux-pci/20191124170207.ga2267...@kroah.com/

You can only go "one level deep" on subdirectories tied to a 'struct
device' and have userspace tools know they are still there.  You can do
that by giving an attribute group a "name" for the directory.

Anything more than that just gets very very messy very quickly and I do
not recommend doing that at all.

thanks,

greg k-h

Re: [PATCH] net:qrtr: fix allocator flag of idr_alloc_u32() in qrtr_port_assign()

[Greg Kroah-Hartman]

On Sat, Mar 27, 2021 at 09:44:37AM +0800, Du Cheng wrote:
> On Fri, Mar 26, 2021 at 10:31:57AM +0100, Greg Kroah-Hartman wrote:
> > On Fri, Mar 26, 2021 at 11:33:45AM +0800, Du Cheng wrote:
> > > change the allocator flag of idr_alloc_u32 from GFP_ATOMIC to
> > > GFP_KERNEL, as GFP_ATOMIC caused BUG: "using smp_processor_id() in
> > > preemptible" as reported by syzkaller.
> > > 
> > > Reported-by: syzbot+3eec59e770685e3dc...@syzkaller.appspotmail.com
> > > Signed-off-by: Du Cheng 
> > > ---
> > > Hi David & Jakub,
> > > 
> > > Although this is a simple fix to make syzkaller happy, I feel that maybe 
> > > a more
> > > proper fix is to convert qrtr_ports from using IDR to radix_tree (which 
> > > is in
> > > fact xarray) ? 
> > > 
> > > I found some previous work done in 2019 by Matthew Wilcox:
> > > https://lore.kernel.org/netdev/20190820223259.22348-1-wi...@infradead.org/t/#mcb60ad4c34e35a6183c7353c8a44ceedfcff297d
> > > but that was not merged as of now. My wild guess is that it was probably
> > > in conflicti with the conversion of radix_tree to xarray during 2020, and 
> > > that
> > > might cause the direct use of xarray in qrtr.c unfavorable.
> > > 
> > > Shall I proceed with converting qrtr_pors to use radix_tree (or just 
> > > xarray)?
> 
> Hi Greg,
> 
> After more scrutiny, this is entirely unnecessary, as the idr structure is
> implemented as a radix_tree, which is, you guess it, xarray :)
> 
> So I looked more closely, and this time I found the culprit of the crash. It 
> was
> due to a unprotected per_cpu access:
> ```
> rtp = this_cpu_ptr(&radix_tree_preloads);
> if (rtp->nr) {
> ret = rtp->nodes;
> rtp->nodes = ret->parent;
> rtp->nr--;
> }
> ```
> inside
> -> radix_tree_node_alloc()
>   -> idr_get_free()
> idr_alloc_u32()
> 
> I tried to wrap the idr_alloc_u32() with disable_preemption() and
> enable_preemption(), and it passed my local and syzbot test.
> 
> More digging reveals that idr routines provide such utilities:
> idr_preload() and idr_preload_end(). They do the exact thing but with 
> additional
> radix_tree bookkeeping. Hence I think this should be favorable than allowing
> the allocation to sleep. The syzbot-passed patch is here:
> https://syzkaller.appspot.com/text?tag=Patch&x=14cf5a26d0
> 
> If it looks good to you, I will send the above patch as V2.

If that resolves the issue, then that's fine with me.

> > Try it and see.  But how would that resolve this issue?  Those other
> > structures would also need to allocate memory at this point in time and
> > you need to tell it if it can sleep or not.
> > 
> > > diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
> > > index edb6ac17ceca..ee42e1e1d4d4 100644
> > > --- a/net/qrtr/qrtr.c
> > > +++ b/net/qrtr/qrtr.c
> > > @@ -722,17 +722,17 @@ static int qrtr_port_assign(struct qrtr_sock *ipc, 
> > > int *port)
> > >   mutex_lock(&qrtr_port_lock);
> > >   if (!*port) {
> > >   min_port = QRTR_MIN_EPH_SOCKET;
> > > - rc = idr_alloc_u32(&qrtr_ports, ipc, &min_port, 
> > > QRTR_MAX_EPH_SOCKET, GFP_ATOMIC);
> > > + rc = idr_alloc_u32(&qrtr_ports, ipc, &min_port, 
> > > QRTR_MAX_EPH_SOCKET, GFP_KERNEL);
> > 
> > Are you sure that you can sleep in this code path?
> There are only 2 other places there the mutex is held, and they seem to be 
> safe,
> but I can't show that comprehensively.
> If I *were* to go with sleeping in idr_alloc_u32, does lockdep a silverbullet 
> to
> prove lock safty?

I do not think lockdep does not test sleeping stuff, it just checks the
order in which locks are held.

You should be able to trace back the code paths here to ensure that
these functions are called in safe context or not, that might be worth
the effort here to make this fix simpler.

thanks,

greg k-h

Re: [PATCH] net:qrtr: fix allocator flag of idr_alloc_u32() in qrtr_port_assign()

[Greg Kroah-Hartman]

On Fri, Mar 26, 2021 at 11:33:45AM +0800, Du Cheng wrote:
> change the allocator flag of idr_alloc_u32 from GFP_ATOMIC to
> GFP_KERNEL, as GFP_ATOMIC caused BUG: "using smp_processor_id() in
> preemptible" as reported by syzkaller.
> 
> Reported-by: syzbot+3eec59e770685e3dc...@syzkaller.appspotmail.com
> Signed-off-by: Du Cheng 
> ---
> Hi David & Jakub,
> 
> Although this is a simple fix to make syzkaller happy, I feel that maybe a 
> more
> proper fix is to convert qrtr_ports from using IDR to radix_tree (which is in
> fact xarray) ? 
> 
> I found some previous work done in 2019 by Matthew Wilcox:
> https://lore.kernel.org/netdev/20190820223259.22348-1-wi...@infradead.org/t/#mcb60ad4c34e35a6183c7353c8a44ceedfcff297d
> but that was not merged as of now. My wild guess is that it was probably
> in conflicti with the conversion of radix_tree to xarray during 2020, and that
> might cause the direct use of xarray in qrtr.c unfavorable.
> 
> Shall I proceed with converting qrtr_pors to use radix_tree (or just xarray)?

Try it and see.  But how would that resolve this issue?  Those other
structures would also need to allocate memory at this point in time and
you need to tell it if it can sleep or not.

> diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
> index edb6ac17ceca..ee42e1e1d4d4 100644
> --- a/net/qrtr/qrtr.c
> +++ b/net/qrtr/qrtr.c
> @@ -722,17 +722,17 @@ static int qrtr_port_assign(struct qrtr_sock *ipc, int 
> *port)
>   mutex_lock(&qrtr_port_lock);
>   if (!*port) {
>   min_port = QRTR_MIN_EPH_SOCKET;
> - rc = idr_alloc_u32(&qrtr_ports, ipc, &min_port, 
> QRTR_MAX_EPH_SOCKET, GFP_ATOMIC);
> + rc = idr_alloc_u32(&qrtr_ports, ipc, &min_port, 
> QRTR_MAX_EPH_SOCKET, GFP_KERNEL);

Are you sure that you can sleep in this code path?

thanks,

greg k-h

Re: [PATCH 5.10 104/157] mptcp: put subflow sock on connect error

[Greg Kroah-Hartman]

On Wed, Mar 24, 2021 at 02:02:06PM +0530, Naresh Kamboju wrote:
> On Mon, 22 Mar 2021 at 18:15, Greg Kroah-Hartman
>  wrote:
> >
> > From: Florian Westphal 
> >
> > [ Upstream commit f07157792c633b528de5fc1dbe2e4ea54f8e09d4 ]
> >
> > mptcp_add_pending_subflow() performs a sock_hold() on the subflow,
> > then adds the subflow to the join list.
> >
> > Without a sock_put the subflow sk won't be freed in case connect() fails.
> >
> > unreferenced object 0x88810c03b100 (size 3000):
> > [..]
> > sk_prot_alloc.isra.0+0x2f/0x110
> > sk_alloc+0x5d/0xc20
> > inet6_create+0x2b7/0xd30
> > __sock_create+0x17f/0x410
> > mptcp_subflow_create_socket+0xff/0x9c0
> > __mptcp_subflow_connect+0x1da/0xaf0
> > mptcp_pm_nl_work+0x6e0/0x1120
> > mptcp_worker+0x508/0x9a0
> >
> > Fixes: 5b950ff4331ddda ("mptcp: link MPC subflow into msk only after 
> > accept")
> > Signed-off-by: Florian Westphal 
> > Signed-off-by: Mat Martineau 
> > Signed-off-by: David S. Miller 
> > Signed-off-by: Sasha Levin 
> 
> I have reported the following warnings and kernel crash on 5.10.26-rc2 [1]
> The bisect reported that issue pointing out to this commit.
> 
> commit 460916534896e6d4f80a37152e0948db33376873
> mptcp: put subflow sock on connect error
> 
> This problem is specific to 5.10.26-rc2.

Thank you for tracking this down!

Re: [PATCH 5.10 104/157] mptcp: put subflow sock on connect error

[Greg Kroah-Hartman]

On Wed, Mar 24, 2021 at 10:04:12AM +0100, Florian Westphal wrote:
> Naresh Kamboju  wrote:
> > On Mon, 22 Mar 2021 at 18:15, Greg Kroah-Hartman
> >  wrote:
> > >
> > > From: Florian Westphal 
> > >
> > > [ Upstream commit f07157792c633b528de5fc1dbe2e4ea54f8e09d4 ]
> > >
> > > mptcp_add_pending_subflow() performs a sock_hold() on the subflow,
> > > then adds the subflow to the join list.
> > >
> > > Without a sock_put the subflow sk won't be freed in case connect() fails.
> > >
> > > unreferenced object 0x88810c03b100 (size 3000):
> > > [..]
> > > sk_prot_alloc.isra.0+0x2f/0x110
> > > sk_alloc+0x5d/0xc20
> > > inet6_create+0x2b7/0xd30
> > > __sock_create+0x17f/0x410
> > > mptcp_subflow_create_socket+0xff/0x9c0
> > > __mptcp_subflow_connect+0x1da/0xaf0
> > > mptcp_pm_nl_work+0x6e0/0x1120
> > > mptcp_worker+0x508/0x9a0
> > >
> > > Fixes: 5b950ff4331ddda ("mptcp: link MPC subflow into msk only after 
> > > accept")
> 
> I don't see this change in 5.10, so why is this fix queued up?
> 
> > I have reported the following warnings and kernel crash on 5.10.26-rc2 [1]
> > The bisect reported that issue pointing out to this commit.
> > 
> > commit 460916534896e6d4f80a37152e0948db33376873
> > mptcp: put subflow sock on connect error
> > 
> > This problem is specific to 5.10.26-rc2.
> > 
> > Warning:
> > 
> > [ 1040.114695] refcount_t: addition on 0; use-after-free.
> > [ 1040.119857] WARNING: CPU: 3 PID: 31925 at
> > /usr/src/kernel/lib/refcount.c:25 refcount_warn_saturate+0xd7/0x100
> > [ 1040.129769] Modules linked in: act_mirred cls_u32 sch_netem sch_etf
> > ip6table_nat xt_nat iptable_nat nf_nat ip6table_filter xt_conntrack
> > nf_conntrack nf_defrag_ipv4 libcrc32c ip6_tables nf_defrag_ipv6 sch_fq
> > iptable_filter xt_mark ip_tables cls_bpf sch_ingress algif_hash
> > x86_pkg_temp_thermal fuse [last unloaded: test_blackhole_dev]
> > [ 1040.159030] CPU: 3 PID: 31925 Comm: mptcp_connect Tainted: G
> > W K   5.10.26-rc2 #1
> > [ 1040.167459] Hardware name: Supermicro SYS-5019S-ML/X11SSH-F, BIOS
> > 2.2 05/23/2018
> > [ 1040.174851] RIP: 0010:refcount_warn_saturate+0xd7/0x100
> > 
> > And
> > 
> > Kernel Panic:
> > -
> > [ 1069.557485] BUG: kernel NULL pointer dereference, address: 
> > 0010
> > [ 1069.564446] #PF: supervisor read access in kernel mode
> > [ 1069.569583] #PF: error_code(0x) - not-present page
> > [ 1069.574714] PGD 0 P4D 0
> > [ 1069.577246] Oops:  [#1] SMP PTI
> > > index 16adba172fb9..591546d0953f 100644
> > > --- a/net/mptcp/subflow.c
> > > +++ b/net/mptcp/subflow.c
> > > @@ -1133,6 +1133,7 @@ int __mptcp_subflow_connect(struct sock *sk, const 
> > > struct mptcp_addr_info *loc,
> > > spin_lock_bh(&msk->join_list_lock);
> > > list_add_tail(&subflow->node, &msk->join_list);
> > > spin_unlock_bh(&msk->join_list_lock);
> > > +   sock_put(mptcp_subflow_tcp_sock(subflow));
> > >
> > > return err;
> 
> Crash is not surprising, the backport puts the socket in the 'success' path
> (list_add_tail).
> 
> I don't see why this is in -stable, the faulty commit is not there?
> 
> The upstream patch is:
> list_del(&subflow->node);
> spin_unlock_bh(&msk->join_list_lock);
> + sock_put(mptcp_subflow_tcp_sock(subflow));
> 
> [ Note the 'list_del', this is in the error unwind path ]

Odd, I think something went wrong with Sasha's scripts.

I've dropped this, and the other two mptcp patches, from the 5.10 queue
and let's see if that helps.  I'll do a new -rc now as well after my
build tests finish...

thanks,

greg k-h

Re: [PATCH] dt-bindings: Drop type references on common properties

[Greg Kroah-Hartman]

On Tue, Mar 16, 2021 at 01:48:58PM -0600, Rob Herring wrote:
> Users of common properties shouldn't have a type definition as the
> common schemas already have one. Drop all the unnecessary type
> references in the tree.
> 
> A meta-schema update to catch these is pending.
> 
> Cc: Nicolas Saenz Julienne 
> Cc: Maxime Ripard 
> Cc: Linus Walleij 
> Cc: Bartosz Golaszewski 
> Cc: Bjorn Andersson 
> Cc: Krzysztof Kozlowski 
> Cc: Marc Kleine-Budde 
> Cc: "David S. Miller" 
> Cc: Jakub Kicinski 
> Cc: Srinivas Kandagatla 
> Cc: Ohad Ben-Cohen 
> Cc: Mark Brown 
> Cc: Cheng-Yi Chiang 
> Cc: Benson Leung 
> Cc: Zhang Rui 
> Cc: Daniel Lezcano 
> Cc: Greg Kroah-Hartman 
> Cc: Stefan Wahren 
> Cc: Masahiro Yamada 
> Cc: Odelu Kukatla 
> Cc: Alex Elder 
> Cc: Suman Anna 
> Cc: Kuninori Morimoto 
> Cc: Dmitry Baryshkov 
> Cc: linux-g...@vger.kernel.org
> Cc: linux...@vger.kernel.org
> Cc: linux-...@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Cc: linux-remotep...@vger.kernel.org
> Cc: alsa-de...@alsa-project.org
> Cc: linux-...@vger.kernel.org
> Signed-off-by: Rob Herring 
> ---
>  .../bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml   | 5 +
>  Documentation/devicetree/bindings/arm/cpus.yaml  | 2 --
>  .../bindings/display/allwinner,sun4i-a10-tcon.yaml   | 1 -
>  .../devicetree/bindings/gpio/socionext,uniphier-gpio.yaml| 3 +--
>  .../devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml  | 1 -
>  .../devicetree/bindings/interconnect/qcom,rpmh.yaml  | 1 -
>  .../bindings/memory-controllers/nvidia,tegra210-emc.yaml | 2 +-
>  Documentation/devicetree/bindings/net/can/fsl,flexcan.yaml   | 1 -
>  Documentation/devicetree/bindings/net/qcom,ipa.yaml  | 1 -
>  Documentation/devicetree/bindings/nvmem/nvmem-consumer.yaml  | 2 --
>  .../devicetree/bindings/remoteproc/ti,omap-remoteproc.yaml   | 2 +-
>  Documentation/devicetree/bindings/sound/ak4642.yaml  | 2 --
>  .../devicetree/bindings/sound/google,cros-ec-codec.yaml  | 2 +-
>  Documentation/devicetree/bindings/sound/renesas,rsnd.yaml| 1 -
>  .../devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml   | 1 -
>  Documentation/devicetree/bindings/usb/usb.yaml   | 1 -
>  16 files changed, 5 insertions(+), 23 deletions(-)

Acked-by: Greg Kroah-Hartman 

Re: [PATCH 4.19-stable 1/3] tcp: annotate tp->copied_seq lockless reads

[Greg Kroah-Hartman]

On Fri, Mar 12, 2021 at 12:33:21AM -0800, Eric Dumazet wrote:
> From: Eric Dumazet 
> 
> [ Upstream commit 7db48e983930285b765743ebd665aecf9850582b ]
> 
> There are few places where we fetch tp->copied_seq while
> this field can change from IRQ or other cpu.
> 
> We need to add READ_ONCE() annotations, and also make
> sure write sides use corresponding WRITE_ONCE() to avoid
> store-tearing.
> 
> Note that tcp_inq_hint() was already using READ_ONCE(tp->copied_seq)
> 
> Signed-off-by: Eric Dumazet 
> Signed-off-by: David S. Miller 

All now queued up, thanks!

greg k-h

Re: [PATCH mlx5-next v7 0/4] Dynamically assign MSI-X vectors count

[Greg Kroah-Hartman]

On Wed, Mar 10, 2021 at 10:10:41PM +0200, Leon Romanovsky wrote:
> On Wed, Mar 10, 2021 at 01:09:06PM -0600, Bjorn Helgaas wrote:
> > On Sun, Mar 07, 2021 at 10:55:24AM -0800, Alexander Duyck wrote:
> > > On Sun, Feb 28, 2021 at 11:55 PM Leon Romanovsky  wrote:
> > > > From: Leon Romanovsky 
> > > >
> > > > @Alexander Duyck, please update me if I can add your ROB tag again
> > > > to the series, because you liked v6 more.
> > > >
> > > > Thanks
> > > >
> > > > -
> > > > Changelog
> > > > v7:
> > > >  * Rebase on top v5.12-rc1
> > > >  * More english fixes
> > > >  * Returned to static sysfs creation model as was implemented in v0/v1.
> 
> <...>
> 
> >   2) Should a VF sysfs file use the PF to implement this?
> >
> >  Can you elaborate on your idea here?  I guess
> >  pci_iov_sysfs_link() makes a "virtfnX" link from the PF to the
> >  VF, and you're thinking we could also make a "virtfnX_msix_count"
> >  in the PF directory?  That's a really interesting idea.
> 
> I want to remind that we are talking about mlx5 devices that support
> upto 255 VFs and they indeed are used to their limits. So seeing 255
> links of virtfnX_msix_count in the same directory looks too much unpleasant
> to me.

255 files are nothing, if that's what the hardware supports, what is the
problem?  If it's "unpleasant", go complain to the hardware designers :)

greg k-h

[PATCH 4.19 20/52] rsi: Move card interrupt handling to RX thread

[Greg Kroah-Hartman]

From: Marek Vasut 

[ Upstream commit 287431463e786766e05e4dc26d0a11d5f8ac8815 ]

The interrupt handling of the RS911x is particularly heavy. For each RX
packet, the card does three SDIO transactions, one to read interrupt
status register, one to RX buffer length, one to read the RX packet(s).
This translates to ~330 uS per one cycle of interrupt handler. In case
there is more incoming traffic, this will be more.

The drivers/mmc/core/sdio_irq.c has the following comment, quote "Just
like traditional hard IRQ handlers, we expect SDIO IRQ handlers to be
quick and to the point, so that the holding of the host lock does not
cover too much work that doesn't require that lock to be held."

The RS911x interrupt handler does not fit that. This patch therefore
changes it such that the entire IRQ handler is moved to the RX thread
instead, and the interrupt handler only wakes the RX thread.

This is OK, because the interrupt handler only does things which can
also be done in the RX thread, that is, it checks for firmware loading
error(s), it checks buffer status, it checks whether a packet arrived
and if so, reads out the packet and passes it to network stack.

Moreover, this change permits removal of a code which allocated an
skbuff only to get 4-byte-aligned buffer, read up to 8kiB of data
into the skbuff, queue this skbuff into local private queue, then in
RX thread, this buffer is dequeued, the data in the skbuff as passed
to the RSI driver core, and the skbuff is deallocated. All this is
replaced by directly calling the RSI driver core with local buffer.

Signed-off-by: Marek Vasut 
Cc: Angus Ainslie 
Cc: David S. Miller 
Cc: Jakub Kicinski 
Cc: Kalle Valo 
Cc: Lee Jones 
Cc: Martin Kepplinger 
Cc: Sebastian Krzyszkowiak 
Cc: Siva Rebbagondla 
Cc: linux-wirel...@vger.kernel.org
Cc: netdev@vger.kernel.org
Tested-by: Martin Kepplinger 
Signed-off-by: Kalle Valo 
Link: https://lore.kernel.org/r/20201103180941.443528-1-ma...@denx.de
Signed-off-by: Sasha Levin 
---
 drivers/net/wireless/rsi/rsi_91x_sdio.c |  6 +--
 drivers/net/wireless/rsi/rsi_91x_sdio_ops.c | 52 ++---
 drivers/net/wireless/rsi/rsi_sdio.h |  8 +---
 3 files changed, 15 insertions(+), 51 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio.c 
b/drivers/net/wireless/rsi/rsi_91x_sdio.c
index 81cc1044532d..f76a360cf1e3 100644
--- a/drivers/net/wireless/rsi/rsi_91x_sdio.c
+++ b/drivers/net/wireless/rsi/rsi_91x_sdio.c
@@ -153,9 +153,7 @@ static void rsi_handle_interrupt(struct sdio_func *function)
if (adapter->priv->fsm_state == FSM_FW_NOT_LOADED)
return;
 
-   dev->sdio_irq_task = current;
-   rsi_interrupt_handler(adapter);
-   dev->sdio_irq_task = NULL;
+   rsi_set_event(&dev->rx_thread.event);
 }
 
 /**
@@ -973,8 +971,6 @@ static int rsi_probe(struct sdio_func *pfunction,
rsi_dbg(ERR_ZONE, "%s: Unable to init rx thrd\n", __func__);
goto fail_kill_thread;
}
-   skb_queue_head_init(&sdev->rx_q.head);
-   sdev->rx_q.num_rx_pkts = 0;
 
sdio_claim_host(pfunction);
if (sdio_claim_irq(pfunction, rsi_handle_interrupt)) {
diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c 
b/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c
index 612c211e21a1..d66ae2f57314 100644
--- a/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c
+++ b/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c
@@ -60,39 +60,20 @@ int rsi_sdio_master_access_msword(struct rsi_hw *adapter, 
u16 ms_word)
return status;
 }
 
+static void rsi_rx_handler(struct rsi_hw *adapter);
+
 void rsi_sdio_rx_thread(struct rsi_common *common)
 {
struct rsi_hw *adapter = common->priv;
struct rsi_91x_sdiodev *sdev = adapter->rsi_dev;
-   struct sk_buff *skb;
-   int status;
 
do {
rsi_wait_event(&sdev->rx_thread.event, EVENT_WAIT_FOREVER);
rsi_reset_event(&sdev->rx_thread.event);
+   rsi_rx_handler(adapter);
+   } while (!atomic_read(&sdev->rx_thread.thread_done));
 
-   while (true) {
-   if (atomic_read(&sdev->rx_thread.thread_done))
-   goto out;
-
-   skb = skb_dequeue(&sdev->rx_q.head);
-   if (!skb)
-   break;
-   if (sdev->rx_q.num_rx_pkts > 0)
-   sdev->rx_q.num_rx_pkts--;
-   status = rsi_read_pkt(common, skb->data, skb->len);
-   if (status) {
-   rsi_dbg(ERR_ZONE, "Failed to read the 
packet\n");
-   dev_kfree_skb(skb);
-   break;
-   }
-   dev_kfree_skb(skb);
-   }
-   } while (1);
-
-out:
rsi_dbg(INFO_ZONE, "%s: Terminated SDIO RX thread\n", __func__);
-   skb_queue_purge(&sdev->rx_q.head);
atomic_inc(&sdev->rx_thread.t

[PATCH 4.19 19/52] rsi: Fix TX EAPOL packet handling against iwlwifi AP

[Greg Kroah-Hartman]

From: Marek Vasut 

[ Upstream commit 65277100caa2f2c62b6f3c4648b90d6f0435f3bc ]

In case RSI9116 SDIO WiFi operates in STA mode against Intel 9260 in AP mode,
the association fails. The former is using wpa_supplicant during association,
the later is set up using hostapd:

iwl$ cat hostapd.conf
interface=wlp1s0
ssid=test
country_code=DE
hw_mode=g
channel=1
wpa=2
wpa_passphrase=test
wpa_key_mgmt=WPA-PSK
iwl$ hostapd -d hostapd.conf

rsi$ wpa_supplicant -i wlan0 -c <(wpa_passphrase test test)

The problem is that the TX EAPOL data descriptor RSI_DESC_REQUIRE_CFM_TO_HOST
flag and extended descriptor EAPOL4_CONFIRM frame type are not set in case the
AP is iwlwifi, because in that case the TX EAPOL packet is 2 bytes shorter.

The downstream vendor driver has this change in place already [1], however
there is no explanation for it, neither is there any commit history from which
such explanation could be obtained.

[1] 
https://github.com/SiliconLabs/RS911X-nLink-OSD/blob/master/rsi/rsi_91x_hal.c#L238

Signed-off-by: Marek Vasut 
Cc: Angus Ainslie 
Cc: David S. Miller 
Cc: Jakub Kicinski 
Cc: Kalle Valo 
Cc: Lee Jones 
Cc: Martin Kepplinger 
Cc: Sebastian Krzyszkowiak 
Cc: Siva Rebbagondla 
Cc: linux-wirel...@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Kalle Valo 
Link: https://lore.kernel.org/r/20201015111616.429220-1-ma...@denx.de
Signed-off-by: Sasha Levin 
---
 drivers/net/wireless/rsi/rsi_91x_hal.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_hal.c 
b/drivers/net/wireless/rsi/rsi_91x_hal.c
index a7b341e95e76..0da95777f1c1 100644
--- a/drivers/net/wireless/rsi/rsi_91x_hal.c
+++ b/drivers/net/wireless/rsi/rsi_91x_hal.c
@@ -238,7 +238,8 @@ int rsi_prepare_data_desc(struct rsi_common *common, struct 
sk_buff *skb)
rsi_set_len_qno(&data_desc->len_qno,
(skb->len - FRAME_DESC_SZ),
RSI_WIFI_MGMT_Q);
-   if ((skb->len - header_size) == EAPOL4_PACKET_LEN) {
+   if (((skb->len - header_size) == EAPOL4_PACKET_LEN) ||
+   ((skb->len - header_size) == EAPOL4_PACKET_LEN - 2)) {
data_desc->misc_flags |=
RSI_DESC_REQUIRE_CFM_TO_HOST;
xtend_desc->confirm_frame_type = EAPOL4_CONFIRM;
-- 
2.30.1

[PATCH 5.4 26/72] rsi: Move card interrupt handling to RX thread

[Greg Kroah-Hartman]

From: Marek Vasut 

[ Upstream commit 287431463e786766e05e4dc26d0a11d5f8ac8815 ]

The interrupt handling of the RS911x is particularly heavy. For each RX
packet, the card does three SDIO transactions, one to read interrupt
status register, one to RX buffer length, one to read the RX packet(s).
This translates to ~330 uS per one cycle of interrupt handler. In case
there is more incoming traffic, this will be more.

The drivers/mmc/core/sdio_irq.c has the following comment, quote "Just
like traditional hard IRQ handlers, we expect SDIO IRQ handlers to be
quick and to the point, so that the holding of the host lock does not
cover too much work that doesn't require that lock to be held."

The RS911x interrupt handler does not fit that. This patch therefore
changes it such that the entire IRQ handler is moved to the RX thread
instead, and the interrupt handler only wakes the RX thread.

This is OK, because the interrupt handler only does things which can
also be done in the RX thread, that is, it checks for firmware loading
error(s), it checks buffer status, it checks whether a packet arrived
and if so, reads out the packet and passes it to network stack.

Moreover, this change permits removal of a code which allocated an
skbuff only to get 4-byte-aligned buffer, read up to 8kiB of data
into the skbuff, queue this skbuff into local private queue, then in
RX thread, this buffer is dequeued, the data in the skbuff as passed
to the RSI driver core, and the skbuff is deallocated. All this is
replaced by directly calling the RSI driver core with local buffer.

Signed-off-by: Marek Vasut 
Cc: Angus Ainslie 
Cc: David S. Miller 
Cc: Jakub Kicinski 
Cc: Kalle Valo 
Cc: Lee Jones 
Cc: Martin Kepplinger 
Cc: Sebastian Krzyszkowiak 
Cc: Siva Rebbagondla 
Cc: linux-wirel...@vger.kernel.org
Cc: netdev@vger.kernel.org
Tested-by: Martin Kepplinger 
Signed-off-by: Kalle Valo 
Link: https://lore.kernel.org/r/20201103180941.443528-1-ma...@denx.de
Signed-off-by: Sasha Levin 
---
 drivers/net/wireless/rsi/rsi_91x_sdio.c |  6 +--
 drivers/net/wireless/rsi/rsi_91x_sdio_ops.c | 52 ++---
 drivers/net/wireless/rsi/rsi_sdio.h |  8 +---
 3 files changed, 15 insertions(+), 51 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio.c 
b/drivers/net/wireless/rsi/rsi_91x_sdio.c
index 1bebba4e8527..d1e8c6593ef5 100644
--- a/drivers/net/wireless/rsi/rsi_91x_sdio.c
+++ b/drivers/net/wireless/rsi/rsi_91x_sdio.c
@@ -153,9 +153,7 @@ static void rsi_handle_interrupt(struct sdio_func *function)
if (adapter->priv->fsm_state == FSM_FW_NOT_LOADED)
return;
 
-   dev->sdio_irq_task = current;
-   rsi_interrupt_handler(adapter);
-   dev->sdio_irq_task = NULL;
+   rsi_set_event(&dev->rx_thread.event);
 }
 
 /**
@@ -1059,8 +1057,6 @@ static int rsi_probe(struct sdio_func *pfunction,
rsi_dbg(ERR_ZONE, "%s: Unable to init rx thrd\n", __func__);
goto fail_kill_thread;
}
-   skb_queue_head_init(&sdev->rx_q.head);
-   sdev->rx_q.num_rx_pkts = 0;
 
sdio_claim_host(pfunction);
if (sdio_claim_irq(pfunction, rsi_handle_interrupt)) {
diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c 
b/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c
index 449f6d23c5e3..7c77b09240da 100644
--- a/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c
+++ b/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c
@@ -60,39 +60,20 @@ int rsi_sdio_master_access_msword(struct rsi_hw *adapter, 
u16 ms_word)
return status;
 }
 
+static void rsi_rx_handler(struct rsi_hw *adapter);
+
 void rsi_sdio_rx_thread(struct rsi_common *common)
 {
struct rsi_hw *adapter = common->priv;
struct rsi_91x_sdiodev *sdev = adapter->rsi_dev;
-   struct sk_buff *skb;
-   int status;
 
do {
rsi_wait_event(&sdev->rx_thread.event, EVENT_WAIT_FOREVER);
rsi_reset_event(&sdev->rx_thread.event);
+   rsi_rx_handler(adapter);
+   } while (!atomic_read(&sdev->rx_thread.thread_done));
 
-   while (true) {
-   if (atomic_read(&sdev->rx_thread.thread_done))
-   goto out;
-
-   skb = skb_dequeue(&sdev->rx_q.head);
-   if (!skb)
-   break;
-   if (sdev->rx_q.num_rx_pkts > 0)
-   sdev->rx_q.num_rx_pkts--;
-   status = rsi_read_pkt(common, skb->data, skb->len);
-   if (status) {
-   rsi_dbg(ERR_ZONE, "Failed to read the 
packet\n");
-   dev_kfree_skb(skb);
-   break;
-   }
-   dev_kfree_skb(skb);
-   }
-   } while (1);
-
-out:
rsi_dbg(INFO_ZONE, "%s: Terminated SDIO RX thread\n", __func__);
-   skb_queue_purge(&sdev->rx_q.head);
atomic_inc(&sdev->rx_thread

[PATCH 5.4 25/72] rsi: Fix TX EAPOL packet handling against iwlwifi AP

[Greg Kroah-Hartman]

From: Marek Vasut 

[ Upstream commit 65277100caa2f2c62b6f3c4648b90d6f0435f3bc ]

In case RSI9116 SDIO WiFi operates in STA mode against Intel 9260 in AP mode,
the association fails. The former is using wpa_supplicant during association,
the later is set up using hostapd:

iwl$ cat hostapd.conf
interface=wlp1s0
ssid=test
country_code=DE
hw_mode=g
channel=1
wpa=2
wpa_passphrase=test
wpa_key_mgmt=WPA-PSK
iwl$ hostapd -d hostapd.conf

rsi$ wpa_supplicant -i wlan0 -c <(wpa_passphrase test test)

The problem is that the TX EAPOL data descriptor RSI_DESC_REQUIRE_CFM_TO_HOST
flag and extended descriptor EAPOL4_CONFIRM frame type are not set in case the
AP is iwlwifi, because in that case the TX EAPOL packet is 2 bytes shorter.

The downstream vendor driver has this change in place already [1], however
there is no explanation for it, neither is there any commit history from which
such explanation could be obtained.

[1] 
https://github.com/SiliconLabs/RS911X-nLink-OSD/blob/master/rsi/rsi_91x_hal.c#L238

Signed-off-by: Marek Vasut 
Cc: Angus Ainslie 
Cc: David S. Miller 
Cc: Jakub Kicinski 
Cc: Kalle Valo 
Cc: Lee Jones 
Cc: Martin Kepplinger 
Cc: Sebastian Krzyszkowiak 
Cc: Siva Rebbagondla 
Cc: linux-wirel...@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Kalle Valo 
Link: https://lore.kernel.org/r/20201015111616.429220-1-ma...@denx.de
Signed-off-by: Sasha Levin 
---
 drivers/net/wireless/rsi/rsi_91x_hal.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_hal.c 
b/drivers/net/wireless/rsi/rsi_91x_hal.c
index 6f8d5f9a9f7e..a07304405b2c 100644
--- a/drivers/net/wireless/rsi/rsi_91x_hal.c
+++ b/drivers/net/wireless/rsi/rsi_91x_hal.c
@@ -248,7 +248,8 @@ int rsi_prepare_data_desc(struct rsi_common *common, struct 
sk_buff *skb)
rsi_set_len_qno(&data_desc->len_qno,
(skb->len - FRAME_DESC_SZ),
RSI_WIFI_MGMT_Q);
-   if ((skb->len - header_size) == EAPOL4_PACKET_LEN) {
+   if (((skb->len - header_size) == EAPOL4_PACKET_LEN) ||
+   ((skb->len - header_size) == EAPOL4_PACKET_LEN - 2)) {
data_desc->misc_flags |=
RSI_DESC_REQUIRE_CFM_TO_HOST;
xtend_desc->confirm_frame_type = EAPOL4_CONFIRM;
-- 
2.30.1

[PATCH 5.10 036/102] rsi: Move card interrupt handling to RX thread

[Greg Kroah-Hartman]

From: Marek Vasut 

[ Upstream commit 287431463e786766e05e4dc26d0a11d5f8ac8815 ]

The interrupt handling of the RS911x is particularly heavy. For each RX
packet, the card does three SDIO transactions, one to read interrupt
status register, one to RX buffer length, one to read the RX packet(s).
This translates to ~330 uS per one cycle of interrupt handler. In case
there is more incoming traffic, this will be more.

The drivers/mmc/core/sdio_irq.c has the following comment, quote "Just
like traditional hard IRQ handlers, we expect SDIO IRQ handlers to be
quick and to the point, so that the holding of the host lock does not
cover too much work that doesn't require that lock to be held."

The RS911x interrupt handler does not fit that. This patch therefore
changes it such that the entire IRQ handler is moved to the RX thread
instead, and the interrupt handler only wakes the RX thread.

This is OK, because the interrupt handler only does things which can
also be done in the RX thread, that is, it checks for firmware loading
error(s), it checks buffer status, it checks whether a packet arrived
and if so, reads out the packet and passes it to network stack.

Moreover, this change permits removal of a code which allocated an
skbuff only to get 4-byte-aligned buffer, read up to 8kiB of data
into the skbuff, queue this skbuff into local private queue, then in
RX thread, this buffer is dequeued, the data in the skbuff as passed
to the RSI driver core, and the skbuff is deallocated. All this is
replaced by directly calling the RSI driver core with local buffer.

Signed-off-by: Marek Vasut 
Cc: Angus Ainslie 
Cc: David S. Miller 
Cc: Jakub Kicinski 
Cc: Kalle Valo 
Cc: Lee Jones 
Cc: Martin Kepplinger 
Cc: Sebastian Krzyszkowiak 
Cc: Siva Rebbagondla 
Cc: linux-wirel...@vger.kernel.org
Cc: netdev@vger.kernel.org
Tested-by: Martin Kepplinger 
Signed-off-by: Kalle Valo 
Link: https://lore.kernel.org/r/20201103180941.443528-1-ma...@denx.de
Signed-off-by: Sasha Levin 
---
 drivers/net/wireless/rsi/rsi_91x_sdio.c |  6 +--
 drivers/net/wireless/rsi/rsi_91x_sdio_ops.c | 52 ++---
 drivers/net/wireless/rsi/rsi_sdio.h |  8 +---
 3 files changed, 15 insertions(+), 51 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio.c 
b/drivers/net/wireless/rsi/rsi_91x_sdio.c
index a7b8684143f4..592e9dadcb55 100644
--- a/drivers/net/wireless/rsi/rsi_91x_sdio.c
+++ b/drivers/net/wireless/rsi/rsi_91x_sdio.c
@@ -153,9 +153,7 @@ static void rsi_handle_interrupt(struct sdio_func *function)
if (adapter->priv->fsm_state == FSM_FW_NOT_LOADED)
return;
 
-   dev->sdio_irq_task = current;
-   rsi_interrupt_handler(adapter);
-   dev->sdio_irq_task = NULL;
+   rsi_set_event(&dev->rx_thread.event);
 }
 
 /**
@@ -1058,8 +1056,6 @@ static int rsi_probe(struct sdio_func *pfunction,
rsi_dbg(ERR_ZONE, "%s: Unable to init rx thrd\n", __func__);
goto fail_kill_thread;
}
-   skb_queue_head_init(&sdev->rx_q.head);
-   sdev->rx_q.num_rx_pkts = 0;
 
sdio_claim_host(pfunction);
if (sdio_claim_irq(pfunction, rsi_handle_interrupt)) {
diff --git a/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c 
b/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c
index 7825c9a889d3..23e709aabd1f 100644
--- a/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c
+++ b/drivers/net/wireless/rsi/rsi_91x_sdio_ops.c
@@ -60,39 +60,20 @@ int rsi_sdio_master_access_msword(struct rsi_hw *adapter, 
u16 ms_word)
return status;
 }
 
+static void rsi_rx_handler(struct rsi_hw *adapter);
+
 void rsi_sdio_rx_thread(struct rsi_common *common)
 {
struct rsi_hw *adapter = common->priv;
struct rsi_91x_sdiodev *sdev = adapter->rsi_dev;
-   struct sk_buff *skb;
-   int status;
 
do {
rsi_wait_event(&sdev->rx_thread.event, EVENT_WAIT_FOREVER);
rsi_reset_event(&sdev->rx_thread.event);
+   rsi_rx_handler(adapter);
+   } while (!atomic_read(&sdev->rx_thread.thread_done));
 
-   while (true) {
-   if (atomic_read(&sdev->rx_thread.thread_done))
-   goto out;
-
-   skb = skb_dequeue(&sdev->rx_q.head);
-   if (!skb)
-   break;
-   if (sdev->rx_q.num_rx_pkts > 0)
-   sdev->rx_q.num_rx_pkts--;
-   status = rsi_read_pkt(common, skb->data, skb->len);
-   if (status) {
-   rsi_dbg(ERR_ZONE, "Failed to read the 
packet\n");
-   dev_kfree_skb(skb);
-   break;
-   }
-   dev_kfree_skb(skb);
-   }
-   } while (1);
-
-out:
rsi_dbg(INFO_ZONE, "%s: Terminated SDIO RX thread\n", __func__);
-   skb_queue_purge(&sdev->rx_q.head);
atomic_inc(&sdev->rx_thread

[PATCH 5.10 035/102] rsi: Fix TX EAPOL packet handling against iwlwifi AP

[Greg Kroah-Hartman]

From: Marek Vasut 

[ Upstream commit 65277100caa2f2c62b6f3c4648b90d6f0435f3bc ]

In case RSI9116 SDIO WiFi operates in STA mode against Intel 9260 in AP mode,
the association fails. The former is using wpa_supplicant during association,
the later is set up using hostapd:

iwl$ cat hostapd.conf
interface=wlp1s0
ssid=test
country_code=DE
hw_mode=g
channel=1
wpa=2
wpa_passphrase=test
wpa_key_mgmt=WPA-PSK
iwl$ hostapd -d hostapd.conf

rsi$ wpa_supplicant -i wlan0 -c <(wpa_passphrase test test)

The problem is that the TX EAPOL data descriptor RSI_DESC_REQUIRE_CFM_TO_HOST
flag and extended descriptor EAPOL4_CONFIRM frame type are not set in case the
AP is iwlwifi, because in that case the TX EAPOL packet is 2 bytes shorter.

The downstream vendor driver has this change in place already [1], however
there is no explanation for it, neither is there any commit history from which
such explanation could be obtained.

[1] 
https://github.com/SiliconLabs/RS911X-nLink-OSD/blob/master/rsi/rsi_91x_hal.c#L238

Signed-off-by: Marek Vasut 
Cc: Angus Ainslie 
Cc: David S. Miller 
Cc: Jakub Kicinski 
Cc: Kalle Valo 
Cc: Lee Jones 
Cc: Martin Kepplinger 
Cc: Sebastian Krzyszkowiak 
Cc: Siva Rebbagondla 
Cc: linux-wirel...@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Kalle Valo 
Link: https://lore.kernel.org/r/20201015111616.429220-1-ma...@denx.de
Signed-off-by: Sasha Levin 
---
 drivers/net/wireless/rsi/rsi_91x_hal.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_hal.c 
b/drivers/net/wireless/rsi/rsi_91x_hal.c
index 3f7e3cfb6f00..ce9892152f4d 100644
--- a/drivers/net/wireless/rsi/rsi_91x_hal.c
+++ b/drivers/net/wireless/rsi/rsi_91x_hal.c
@@ -248,7 +248,8 @@ int rsi_prepare_data_desc(struct rsi_common *common, struct 
sk_buff *skb)
rsi_set_len_qno(&data_desc->len_qno,
(skb->len - FRAME_DESC_SZ),
RSI_WIFI_MGMT_Q);
-   if ((skb->len - header_size) == EAPOL4_PACKET_LEN) {
+   if (((skb->len - header_size) == EAPOL4_PACKET_LEN) ||
+   ((skb->len - header_size) == EAPOL4_PACKET_LEN - 2)) {
data_desc->misc_flags |=
RSI_DESC_REQUIRE_CFM_TO_HOST;
xtend_desc->confirm_frame_type = EAPOL4_CONFIRM;
-- 
2.30.1

[PATCH 5.11 409/775] tracepoint: Do not fail unregistering a probe due to memory failure

[Greg Kroah-Hartman]

From: Steven Rostedt (VMware) 

[ Upstream commit befe6d946551d65cddbd32b9cb0170b0249fd5ed ]

The list of tracepoint callbacks is managed by an array that is protected
by RCU. To update this array, a new array is allocated, the updates are
copied over to the new array, and then the list of functions for the
tracepoint is switched over to the new array. After a completion of an RCU
grace period, the old array is freed.

This process happens for both adding a callback as well as removing one.
But on removing a callback, if the new array fails to be allocated, the
callback is not removed, and may be used after it is freed by the clients
of the tracepoint.

There's really no reason to fail if the allocation for a new array fails
when removing a function. Instead, the function can simply be replaced by a
stub function that could be cleaned up on the next modification of the
array. That is, instead of calling the function registered to the
tracepoint, it would call a stub function in its place.

Link: https://lore.kernel.org/r/20201115055256.65625-1-mmull...@mmlx.us
Link: https://lore.kernel.org/r/20201116175107.02db3...@gandalf.local.home
Link: https://lore.kernel.org/r/20201117211836.54aca...@oasis.local.home
Link: https://lkml.kernel.org/r/20201118093405.7a6d2...@gandalf.local.home

[ Note, this version does use undefined compiler behavior (assuming that
  a stub function with no parameters or return, can be called by a location
  that thinks it has parameters but still no return value. Static calls
  do the same thing, so this trick is not without precedent.

  There's another solution that uses RCU tricks and is more complex, but
  can be an alternative if this solution becomes an issue.

  Link: https://lore.kernel.org/lkml/20210127170721.58bce...@gandalf.local.home/
]

Cc: Peter Zijlstra 
Cc: Josh Poimboeuf 
Cc: Mathieu Desnoyers 
Cc: Ingo Molnar 
Cc: Alexei Starovoitov 
Cc: Daniel Borkmann 
Cc: Dmitry Vyukov 
Cc: Martin KaFai Lau 
Cc: Song Liu 
Cc: Yonghong Song 
Cc: Andrii Nakryiko 
Cc: John Fastabend 
Cc: KP Singh 
Cc: netdev 
Cc: bpf 
Cc: Kees Cook 
Cc: Florian Weimer 
Fixes: 97e1c18e8d17b ("tracing: Kernel Tracepoints")
Reported-by: syzbot+83aa762ef23b6f0d1...@syzkaller.appspotmail.com
Reported-by: syzbot+d29e58bb557324e55...@syzkaller.appspotmail.com
Reported-by: Matt Mullins 
Signed-off-by: Steven Rostedt (VMware) 
Tested-by: Matt Mullins 
Signed-off-by: Sasha Levin 
---
 kernel/tracepoint.c | 80 -
 1 file changed, 64 insertions(+), 16 deletions(-)

diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 7261fa0f5e3cc..e8f20ae29c18f 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -53,6 +53,12 @@ struct tp_probes {
struct tracepoint_func probes[];
 };
 
+/* Called in removal of a func but failed to allocate a new tp_funcs */
+static void tp_stub_func(void)
+{
+   return;
+}
+
 static inline void *allocate_probes(int count)
 {
struct tp_probes *p  = kmalloc(struct_size(p, probes, count),
@@ -131,6 +137,7 @@ func_add(struct tracepoint_func **funcs, struct 
tracepoint_func *tp_func,
 {
struct tracepoint_func *old, *new;
int nr_probes = 0;
+   int stub_funcs = 0;
int pos = -1;
 
if (WARN_ON(!tp_func->func))
@@ -147,14 +154,34 @@ func_add(struct tracepoint_func **funcs, struct 
tracepoint_func *tp_func,
if (old[nr_probes].func == tp_func->func &&
old[nr_probes].data == tp_func->data)
return ERR_PTR(-EEXIST);
+   if (old[nr_probes].func == tp_stub_func)
+   stub_funcs++;
}
}
-   /* + 2 : one for new probe, one for NULL func */
-   new = allocate_probes(nr_probes + 2);
+   /* + 2 : one for new probe, one for NULL func - stub functions */
+   new = allocate_probes(nr_probes + 2 - stub_funcs);
if (new == NULL)
return ERR_PTR(-ENOMEM);
if (old) {
-   if (pos < 0) {
+   if (stub_funcs) {
+   /* Need to copy one at a time to remove stubs */
+   int probes = 0;
+
+   pos = -1;
+   for (nr_probes = 0; old[nr_probes].func; nr_probes++) {
+   if (old[nr_probes].func == tp_stub_func)
+   continue;
+   if (pos < 0 && old[nr_probes].prio < prio)
+   pos = probes++;
+   new[probes++] = old[nr_probes];
+   }
+   nr_probes = probes;
+   if (pos < 0)
+   pos = probes;
+   else
+   nr_probes--; /* Account for insertion */
+
+   } else if (pos < 0) {
pos = nr_probes;

[PATCH 5.10 341/663] tracepoint: Do not fail unregistering a probe due to memory failure

[Greg Kroah-Hartman]

From: Steven Rostedt (VMware) 

[ Upstream commit befe6d946551d65cddbd32b9cb0170b0249fd5ed ]

The list of tracepoint callbacks is managed by an array that is protected
by RCU. To update this array, a new array is allocated, the updates are
copied over to the new array, and then the list of functions for the
tracepoint is switched over to the new array. After a completion of an RCU
grace period, the old array is freed.

This process happens for both adding a callback as well as removing one.
But on removing a callback, if the new array fails to be allocated, the
callback is not removed, and may be used after it is freed by the clients
of the tracepoint.

There's really no reason to fail if the allocation for a new array fails
when removing a function. Instead, the function can simply be replaced by a
stub function that could be cleaned up on the next modification of the
array. That is, instead of calling the function registered to the
tracepoint, it would call a stub function in its place.

Link: https://lore.kernel.org/r/20201115055256.65625-1-mmull...@mmlx.us
Link: https://lore.kernel.org/r/20201116175107.02db3...@gandalf.local.home
Link: https://lore.kernel.org/r/20201117211836.54aca...@oasis.local.home
Link: https://lkml.kernel.org/r/20201118093405.7a6d2...@gandalf.local.home

[ Note, this version does use undefined compiler behavior (assuming that
  a stub function with no parameters or return, can be called by a location
  that thinks it has parameters but still no return value. Static calls
  do the same thing, so this trick is not without precedent.

  There's another solution that uses RCU tricks and is more complex, but
  can be an alternative if this solution becomes an issue.

  Link: https://lore.kernel.org/lkml/20210127170721.58bce...@gandalf.local.home/
]

Cc: Peter Zijlstra 
Cc: Josh Poimboeuf 
Cc: Mathieu Desnoyers 
Cc: Ingo Molnar 
Cc: Alexei Starovoitov 
Cc: Daniel Borkmann 
Cc: Dmitry Vyukov 
Cc: Martin KaFai Lau 
Cc: Song Liu 
Cc: Yonghong Song 
Cc: Andrii Nakryiko 
Cc: John Fastabend 
Cc: KP Singh 
Cc: netdev 
Cc: bpf 
Cc: Kees Cook 
Cc: Florian Weimer 
Fixes: 97e1c18e8d17b ("tracing: Kernel Tracepoints")
Reported-by: syzbot+83aa762ef23b6f0d1...@syzkaller.appspotmail.com
Reported-by: syzbot+d29e58bb557324e55...@syzkaller.appspotmail.com
Reported-by: Matt Mullins 
Signed-off-by: Steven Rostedt (VMware) 
Tested-by: Matt Mullins 
Signed-off-by: Sasha Levin 
---
 kernel/tracepoint.c | 80 -
 1 file changed, 64 insertions(+), 16 deletions(-)

diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 3f659f8550741..3e261482296cf 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -53,6 +53,12 @@ struct tp_probes {
struct tracepoint_func probes[];
 };
 
+/* Called in removal of a func but failed to allocate a new tp_funcs */
+static void tp_stub_func(void)
+{
+   return;
+}
+
 static inline void *allocate_probes(int count)
 {
struct tp_probes *p  = kmalloc(struct_size(p, probes, count),
@@ -131,6 +137,7 @@ func_add(struct tracepoint_func **funcs, struct 
tracepoint_func *tp_func,
 {
struct tracepoint_func *old, *new;
int nr_probes = 0;
+   int stub_funcs = 0;
int pos = -1;
 
if (WARN_ON(!tp_func->func))
@@ -147,14 +154,34 @@ func_add(struct tracepoint_func **funcs, struct 
tracepoint_func *tp_func,
if (old[nr_probes].func == tp_func->func &&
old[nr_probes].data == tp_func->data)
return ERR_PTR(-EEXIST);
+   if (old[nr_probes].func == tp_stub_func)
+   stub_funcs++;
}
}
-   /* + 2 : one for new probe, one for NULL func */
-   new = allocate_probes(nr_probes + 2);
+   /* + 2 : one for new probe, one for NULL func - stub functions */
+   new = allocate_probes(nr_probes + 2 - stub_funcs);
if (new == NULL)
return ERR_PTR(-ENOMEM);
if (old) {
-   if (pos < 0) {
+   if (stub_funcs) {
+   /* Need to copy one at a time to remove stubs */
+   int probes = 0;
+
+   pos = -1;
+   for (nr_probes = 0; old[nr_probes].func; nr_probes++) {
+   if (old[nr_probes].func == tp_stub_func)
+   continue;
+   if (pos < 0 && old[nr_probes].prio < prio)
+   pos = probes++;
+   new[probes++] = old[nr_probes];
+   }
+   nr_probes = probes;
+   if (pos < 0)
+   pos = probes;
+   else
+   nr_probes--; /* Account for insertion */
+
+   } else if (pos < 0) {
pos = nr_probes;

[PATCH 5.4 176/340] tracepoint: Do not fail unregistering a probe due to memory failure

[Greg Kroah-Hartman]

From: Steven Rostedt (VMware) 

[ Upstream commit befe6d946551d65cddbd32b9cb0170b0249fd5ed ]

The list of tracepoint callbacks is managed by an array that is protected
by RCU. To update this array, a new array is allocated, the updates are
copied over to the new array, and then the list of functions for the
tracepoint is switched over to the new array. After a completion of an RCU
grace period, the old array is freed.

This process happens for both adding a callback as well as removing one.
But on removing a callback, if the new array fails to be allocated, the
callback is not removed, and may be used after it is freed by the clients
of the tracepoint.

There's really no reason to fail if the allocation for a new array fails
when removing a function. Instead, the function can simply be replaced by a
stub function that could be cleaned up on the next modification of the
array. That is, instead of calling the function registered to the
tracepoint, it would call a stub function in its place.

Link: https://lore.kernel.org/r/20201115055256.65625-1-mmull...@mmlx.us
Link: https://lore.kernel.org/r/20201116175107.02db3...@gandalf.local.home
Link: https://lore.kernel.org/r/20201117211836.54aca...@oasis.local.home
Link: https://lkml.kernel.org/r/20201118093405.7a6d2...@gandalf.local.home

[ Note, this version does use undefined compiler behavior (assuming that
  a stub function with no parameters or return, can be called by a location
  that thinks it has parameters but still no return value. Static calls
  do the same thing, so this trick is not without precedent.

  There's another solution that uses RCU tricks and is more complex, but
  can be an alternative if this solution becomes an issue.

  Link: https://lore.kernel.org/lkml/20210127170721.58bce...@gandalf.local.home/
]

Cc: Peter Zijlstra 
Cc: Josh Poimboeuf 
Cc: Mathieu Desnoyers 
Cc: Ingo Molnar 
Cc: Alexei Starovoitov 
Cc: Daniel Borkmann 
Cc: Dmitry Vyukov 
Cc: Martin KaFai Lau 
Cc: Song Liu 
Cc: Yonghong Song 
Cc: Andrii Nakryiko 
Cc: John Fastabend 
Cc: KP Singh 
Cc: netdev 
Cc: bpf 
Cc: Kees Cook 
Cc: Florian Weimer 
Fixes: 97e1c18e8d17b ("tracing: Kernel Tracepoints")
Reported-by: syzbot+83aa762ef23b6f0d1...@syzkaller.appspotmail.com
Reported-by: syzbot+d29e58bb557324e55...@syzkaller.appspotmail.com
Reported-by: Matt Mullins 
Signed-off-by: Steven Rostedt (VMware) 
Tested-by: Matt Mullins 
Signed-off-by: Sasha Levin 
---
 kernel/tracepoint.c | 80 -
 1 file changed, 64 insertions(+), 16 deletions(-)

diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index 73956eaff8a9c..be51df4508cbe 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -53,6 +53,12 @@ struct tp_probes {
struct tracepoint_func probes[0];
 };
 
+/* Called in removal of a func but failed to allocate a new tp_funcs */
+static void tp_stub_func(void)
+{
+   return;
+}
+
 static inline void *allocate_probes(int count)
 {
struct tp_probes *p  = kmalloc(struct_size(p, probes, count),
@@ -131,6 +137,7 @@ func_add(struct tracepoint_func **funcs, struct 
tracepoint_func *tp_func,
 {
struct tracepoint_func *old, *new;
int nr_probes = 0;
+   int stub_funcs = 0;
int pos = -1;
 
if (WARN_ON(!tp_func->func))
@@ -147,14 +154,34 @@ func_add(struct tracepoint_func **funcs, struct 
tracepoint_func *tp_func,
if (old[nr_probes].func == tp_func->func &&
old[nr_probes].data == tp_func->data)
return ERR_PTR(-EEXIST);
+   if (old[nr_probes].func == tp_stub_func)
+   stub_funcs++;
}
}
-   /* + 2 : one for new probe, one for NULL func */
-   new = allocate_probes(nr_probes + 2);
+   /* + 2 : one for new probe, one for NULL func - stub functions */
+   new = allocate_probes(nr_probes + 2 - stub_funcs);
if (new == NULL)
return ERR_PTR(-ENOMEM);
if (old) {
-   if (pos < 0) {
+   if (stub_funcs) {
+   /* Need to copy one at a time to remove stubs */
+   int probes = 0;
+
+   pos = -1;
+   for (nr_probes = 0; old[nr_probes].func; nr_probes++) {
+   if (old[nr_probes].func == tp_stub_func)
+   continue;
+   if (pos < 0 && old[nr_probes].prio < prio)
+   pos = probes++;
+   new[probes++] = old[nr_probes];
+   }
+   nr_probes = probes;
+   if (pos < 0)
+   pos = probes;
+   else
+   nr_probes--; /* Account for insertion */
+
+   } else if (pos < 0) {
pos = nr_probes;
   

[PATCH 4.19 139/247] tracepoint: Do not fail unregistering a probe due to memory failure

[Greg Kroah-Hartman]

From: Steven Rostedt (VMware) 

[ Upstream commit befe6d946551d65cddbd32b9cb0170b0249fd5ed ]

The list of tracepoint callbacks is managed by an array that is protected
by RCU. To update this array, a new array is allocated, the updates are
copied over to the new array, and then the list of functions for the
tracepoint is switched over to the new array. After a completion of an RCU
grace period, the old array is freed.

This process happens for both adding a callback as well as removing one.
But on removing a callback, if the new array fails to be allocated, the
callback is not removed, and may be used after it is freed by the clients
of the tracepoint.

There's really no reason to fail if the allocation for a new array fails
when removing a function. Instead, the function can simply be replaced by a
stub function that could be cleaned up on the next modification of the
array. That is, instead of calling the function registered to the
tracepoint, it would call a stub function in its place.

Link: https://lore.kernel.org/r/20201115055256.65625-1-mmull...@mmlx.us
Link: https://lore.kernel.org/r/20201116175107.02db3...@gandalf.local.home
Link: https://lore.kernel.org/r/20201117211836.54aca...@oasis.local.home
Link: https://lkml.kernel.org/r/20201118093405.7a6d2...@gandalf.local.home

[ Note, this version does use undefined compiler behavior (assuming that
  a stub function with no parameters or return, can be called by a location
  that thinks it has parameters but still no return value. Static calls
  do the same thing, so this trick is not without precedent.

  There's another solution that uses RCU tricks and is more complex, but
  can be an alternative if this solution becomes an issue.

  Link: https://lore.kernel.org/lkml/20210127170721.58bce...@gandalf.local.home/
]

Cc: Peter Zijlstra 
Cc: Josh Poimboeuf 
Cc: Mathieu Desnoyers 
Cc: Ingo Molnar 
Cc: Alexei Starovoitov 
Cc: Daniel Borkmann 
Cc: Dmitry Vyukov 
Cc: Martin KaFai Lau 
Cc: Song Liu 
Cc: Yonghong Song 
Cc: Andrii Nakryiko 
Cc: John Fastabend 
Cc: KP Singh 
Cc: netdev 
Cc: bpf 
Cc: Kees Cook 
Cc: Florian Weimer 
Fixes: 97e1c18e8d17b ("tracing: Kernel Tracepoints")
Reported-by: syzbot+83aa762ef23b6f0d1...@syzkaller.appspotmail.com
Reported-by: syzbot+d29e58bb557324e55...@syzkaller.appspotmail.com
Reported-by: Matt Mullins 
Signed-off-by: Steven Rostedt (VMware) 
Tested-by: Matt Mullins 
Signed-off-by: Sasha Levin 
---
 kernel/tracepoint.c | 80 -
 1 file changed, 64 insertions(+), 16 deletions(-)

diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index a3be42304485f..d5ce692319128 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -66,6 +66,12 @@ struct tp_probes {
struct tracepoint_func probes[0];
 };
 
+/* Called in removal of a func but failed to allocate a new tp_funcs */
+static void tp_stub_func(void)
+{
+   return;
+}
+
 static inline void *allocate_probes(int count)
 {
struct tp_probes *p  = kmalloc(count * sizeof(struct tracepoint_func)
@@ -144,6 +150,7 @@ func_add(struct tracepoint_func **funcs, struct 
tracepoint_func *tp_func,
 {
struct tracepoint_func *old, *new;
int nr_probes = 0;
+   int stub_funcs = 0;
int pos = -1;
 
if (WARN_ON(!tp_func->func))
@@ -160,14 +167,34 @@ func_add(struct tracepoint_func **funcs, struct 
tracepoint_func *tp_func,
if (old[nr_probes].func == tp_func->func &&
old[nr_probes].data == tp_func->data)
return ERR_PTR(-EEXIST);
+   if (old[nr_probes].func == tp_stub_func)
+   stub_funcs++;
}
}
-   /* + 2 : one for new probe, one for NULL func */
-   new = allocate_probes(nr_probes + 2);
+   /* + 2 : one for new probe, one for NULL func - stub functions */
+   new = allocate_probes(nr_probes + 2 - stub_funcs);
if (new == NULL)
return ERR_PTR(-ENOMEM);
if (old) {
-   if (pos < 0) {
+   if (stub_funcs) {
+   /* Need to copy one at a time to remove stubs */
+   int probes = 0;
+
+   pos = -1;
+   for (nr_probes = 0; old[nr_probes].func; nr_probes++) {
+   if (old[nr_probes].func == tp_stub_func)
+   continue;
+   if (pos < 0 && old[nr_probes].prio < prio)
+   pos = probes++;
+   new[probes++] = old[nr_probes];
+   }
+   nr_probes = probes;
+   if (pos < 0)
+   pos = probes;
+   else
+   nr_probes--; /* Account for insertion */
+
+   } else if (pos < 0) {
pos = nr_probes;
   

[PATCH 4.14 091/176] tracepoint: Do not fail unregistering a probe due to memory failure

[Greg Kroah-Hartman]

From: Steven Rostedt (VMware) 

[ Upstream commit befe6d946551d65cddbd32b9cb0170b0249fd5ed ]

The list of tracepoint callbacks is managed by an array that is protected
by RCU. To update this array, a new array is allocated, the updates are
copied over to the new array, and then the list of functions for the
tracepoint is switched over to the new array. After a completion of an RCU
grace period, the old array is freed.

This process happens for both adding a callback as well as removing one.
But on removing a callback, if the new array fails to be allocated, the
callback is not removed, and may be used after it is freed by the clients
of the tracepoint.

There's really no reason to fail if the allocation for a new array fails
when removing a function. Instead, the function can simply be replaced by a
stub function that could be cleaned up on the next modification of the
array. That is, instead of calling the function registered to the
tracepoint, it would call a stub function in its place.

Link: https://lore.kernel.org/r/20201115055256.65625-1-mmull...@mmlx.us
Link: https://lore.kernel.org/r/20201116175107.02db3...@gandalf.local.home
Link: https://lore.kernel.org/r/20201117211836.54aca...@oasis.local.home
Link: https://lkml.kernel.org/r/20201118093405.7a6d2...@gandalf.local.home

[ Note, this version does use undefined compiler behavior (assuming that
  a stub function with no parameters or return, can be called by a location
  that thinks it has parameters but still no return value. Static calls
  do the same thing, so this trick is not without precedent.

  There's another solution that uses RCU tricks and is more complex, but
  can be an alternative if this solution becomes an issue.

  Link: https://lore.kernel.org/lkml/20210127170721.58bce...@gandalf.local.home/
]

Cc: Peter Zijlstra 
Cc: Josh Poimboeuf 
Cc: Mathieu Desnoyers 
Cc: Ingo Molnar 
Cc: Alexei Starovoitov 
Cc: Daniel Borkmann 
Cc: Dmitry Vyukov 
Cc: Martin KaFai Lau 
Cc: Song Liu 
Cc: Yonghong Song 
Cc: Andrii Nakryiko 
Cc: John Fastabend 
Cc: KP Singh 
Cc: netdev 
Cc: bpf 
Cc: Kees Cook 
Cc: Florian Weimer 
Fixes: 97e1c18e8d17b ("tracing: Kernel Tracepoints")
Reported-by: syzbot+83aa762ef23b6f0d1...@syzkaller.appspotmail.com
Reported-by: syzbot+d29e58bb557324e55...@syzkaller.appspotmail.com
Reported-by: Matt Mullins 
Signed-off-by: Steven Rostedt (VMware) 
Tested-by: Matt Mullins 
Signed-off-by: Sasha Levin 
---
 kernel/tracepoint.c | 80 -
 1 file changed, 64 insertions(+), 16 deletions(-)

diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index a170d83043a5a..b65b2e7fd8507 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -60,6 +60,12 @@ struct tp_probes {
struct tracepoint_func probes[0];
 };
 
+/* Called in removal of a func but failed to allocate a new tp_funcs */
+static void tp_stub_func(void)
+{
+   return;
+}
+
 static inline void *allocate_probes(int count)
 {
struct tp_probes *p  = kmalloc(count * sizeof(struct tracepoint_func)
@@ -98,6 +104,7 @@ func_add(struct tracepoint_func **funcs, struct 
tracepoint_func *tp_func,
 {
struct tracepoint_func *old, *new;
int nr_probes = 0;
+   int stub_funcs = 0;
int pos = -1;
 
if (WARN_ON(!tp_func->func))
@@ -114,14 +121,34 @@ func_add(struct tracepoint_func **funcs, struct 
tracepoint_func *tp_func,
if (old[nr_probes].func == tp_func->func &&
old[nr_probes].data == tp_func->data)
return ERR_PTR(-EEXIST);
+   if (old[nr_probes].func == tp_stub_func)
+   stub_funcs++;
}
}
-   /* + 2 : one for new probe, one for NULL func */
-   new = allocate_probes(nr_probes + 2);
+   /* + 2 : one for new probe, one for NULL func - stub functions */
+   new = allocate_probes(nr_probes + 2 - stub_funcs);
if (new == NULL)
return ERR_PTR(-ENOMEM);
if (old) {
-   if (pos < 0) {
+   if (stub_funcs) {
+   /* Need to copy one at a time to remove stubs */
+   int probes = 0;
+
+   pos = -1;
+   for (nr_probes = 0; old[nr_probes].func; nr_probes++) {
+   if (old[nr_probes].func == tp_stub_func)
+   continue;
+   if (pos < 0 && old[nr_probes].prio < prio)
+   pos = probes++;
+   new[probes++] = old[nr_probes];
+   }
+   nr_probes = probes;
+   if (pos < 0)
+   pos = probes;
+   else
+   nr_probes--; /* Account for insertion */
+
+   } else if (pos < 0) {
pos = nr_probes;

[PATCH 4.9 064/134] tracepoint: Do not fail unregistering a probe due to memory failure

[Greg Kroah-Hartman]

From: Steven Rostedt (VMware) 

[ Upstream commit befe6d946551d65cddbd32b9cb0170b0249fd5ed ]

The list of tracepoint callbacks is managed by an array that is protected
by RCU. To update this array, a new array is allocated, the updates are
copied over to the new array, and then the list of functions for the
tracepoint is switched over to the new array. After a completion of an RCU
grace period, the old array is freed.

This process happens for both adding a callback as well as removing one.
But on removing a callback, if the new array fails to be allocated, the
callback is not removed, and may be used after it is freed by the clients
of the tracepoint.

There's really no reason to fail if the allocation for a new array fails
when removing a function. Instead, the function can simply be replaced by a
stub function that could be cleaned up on the next modification of the
array. That is, instead of calling the function registered to the
tracepoint, it would call a stub function in its place.

Link: https://lore.kernel.org/r/20201115055256.65625-1-mmull...@mmlx.us
Link: https://lore.kernel.org/r/20201116175107.02db3...@gandalf.local.home
Link: https://lore.kernel.org/r/20201117211836.54aca...@oasis.local.home
Link: https://lkml.kernel.org/r/20201118093405.7a6d2...@gandalf.local.home

[ Note, this version does use undefined compiler behavior (assuming that
  a stub function with no parameters or return, can be called by a location
  that thinks it has parameters but still no return value. Static calls
  do the same thing, so this trick is not without precedent.

  There's another solution that uses RCU tricks and is more complex, but
  can be an alternative if this solution becomes an issue.

  Link: https://lore.kernel.org/lkml/20210127170721.58bce...@gandalf.local.home/
]

Cc: Peter Zijlstra 
Cc: Josh Poimboeuf 
Cc: Mathieu Desnoyers 
Cc: Ingo Molnar 
Cc: Alexei Starovoitov 
Cc: Daniel Borkmann 
Cc: Dmitry Vyukov 
Cc: Martin KaFai Lau 
Cc: Song Liu 
Cc: Yonghong Song 
Cc: Andrii Nakryiko 
Cc: John Fastabend 
Cc: KP Singh 
Cc: netdev 
Cc: bpf 
Cc: Kees Cook 
Cc: Florian Weimer 
Fixes: 97e1c18e8d17b ("tracing: Kernel Tracepoints")
Reported-by: syzbot+83aa762ef23b6f0d1...@syzkaller.appspotmail.com
Reported-by: syzbot+d29e58bb557324e55...@syzkaller.appspotmail.com
Reported-by: Matt Mullins 
Signed-off-by: Steven Rostedt (VMware) 
Tested-by: Matt Mullins 
Signed-off-by: Sasha Levin 
---
 kernel/tracepoint.c | 80 -
 1 file changed, 64 insertions(+), 16 deletions(-)

diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index c8e7cc0e6ff6e..88ae873ee6cf3 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -59,6 +59,12 @@ struct tp_probes {
struct tracepoint_func probes[0];
 };
 
+/* Called in removal of a func but failed to allocate a new tp_funcs */
+static void tp_stub_func(void)
+{
+   return;
+}
+
 static inline void *allocate_probes(int count)
 {
struct tp_probes *p  = kmalloc(count * sizeof(struct tracepoint_func)
@@ -97,6 +103,7 @@ func_add(struct tracepoint_func **funcs, struct 
tracepoint_func *tp_func,
 {
struct tracepoint_func *old, *new;
int nr_probes = 0;
+   int stub_funcs = 0;
int pos = -1;
 
if (WARN_ON(!tp_func->func))
@@ -113,14 +120,34 @@ func_add(struct tracepoint_func **funcs, struct 
tracepoint_func *tp_func,
if (old[nr_probes].func == tp_func->func &&
old[nr_probes].data == tp_func->data)
return ERR_PTR(-EEXIST);
+   if (old[nr_probes].func == tp_stub_func)
+   stub_funcs++;
}
}
-   /* + 2 : one for new probe, one for NULL func */
-   new = allocate_probes(nr_probes + 2);
+   /* + 2 : one for new probe, one for NULL func - stub functions */
+   new = allocate_probes(nr_probes + 2 - stub_funcs);
if (new == NULL)
return ERR_PTR(-ENOMEM);
if (old) {
-   if (pos < 0) {
+   if (stub_funcs) {
+   /* Need to copy one at a time to remove stubs */
+   int probes = 0;
+
+   pos = -1;
+   for (nr_probes = 0; old[nr_probes].func; nr_probes++) {
+   if (old[nr_probes].func == tp_stub_func)
+   continue;
+   if (pos < 0 && old[nr_probes].prio < prio)
+   pos = probes++;
+   new[probes++] = old[nr_probes];
+   }
+   nr_probes = probes;
+   if (pos < 0)
+   pos = probes;
+   else
+   nr_probes--; /* Account for insertion */
+
+   } else if (pos < 0) {
pos = nr_probes;

[PATCH 4.4 46/93] tracepoint: Do not fail unregistering a probe due to memory failure

[Greg Kroah-Hartman]

From: Steven Rostedt (VMware) 

[ Upstream commit befe6d946551d65cddbd32b9cb0170b0249fd5ed ]

The list of tracepoint callbacks is managed by an array that is protected
by RCU. To update this array, a new array is allocated, the updates are
copied over to the new array, and then the list of functions for the
tracepoint is switched over to the new array. After a completion of an RCU
grace period, the old array is freed.

This process happens for both adding a callback as well as removing one.
But on removing a callback, if the new array fails to be allocated, the
callback is not removed, and may be used after it is freed by the clients
of the tracepoint.

There's really no reason to fail if the allocation for a new array fails
when removing a function. Instead, the function can simply be replaced by a
stub function that could be cleaned up on the next modification of the
array. That is, instead of calling the function registered to the
tracepoint, it would call a stub function in its place.

Link: https://lore.kernel.org/r/20201115055256.65625-1-mmull...@mmlx.us
Link: https://lore.kernel.org/r/20201116175107.02db3...@gandalf.local.home
Link: https://lore.kernel.org/r/20201117211836.54aca...@oasis.local.home
Link: https://lkml.kernel.org/r/20201118093405.7a6d2...@gandalf.local.home

[ Note, this version does use undefined compiler behavior (assuming that
  a stub function with no parameters or return, can be called by a location
  that thinks it has parameters but still no return value. Static calls
  do the same thing, so this trick is not without precedent.

  There's another solution that uses RCU tricks and is more complex, but
  can be an alternative if this solution becomes an issue.

  Link: https://lore.kernel.org/lkml/20210127170721.58bce...@gandalf.local.home/
]

Cc: Peter Zijlstra 
Cc: Josh Poimboeuf 
Cc: Mathieu Desnoyers 
Cc: Ingo Molnar 
Cc: Alexei Starovoitov 
Cc: Daniel Borkmann 
Cc: Dmitry Vyukov 
Cc: Martin KaFai Lau 
Cc: Song Liu 
Cc: Yonghong Song 
Cc: Andrii Nakryiko 
Cc: John Fastabend 
Cc: KP Singh 
Cc: netdev 
Cc: bpf 
Cc: Kees Cook 
Cc: Florian Weimer 
Fixes: 97e1c18e8d17b ("tracing: Kernel Tracepoints")
Reported-by: syzbot+83aa762ef23b6f0d1...@syzkaller.appspotmail.com
Reported-by: syzbot+d29e58bb557324e55...@syzkaller.appspotmail.com
Reported-by: Matt Mullins 
Signed-off-by: Steven Rostedt (VMware) 
Tested-by: Matt Mullins 
Signed-off-by: Sasha Levin 
---
 kernel/tracepoint.c | 80 -
 1 file changed, 64 insertions(+), 16 deletions(-)

diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c
index eda85bbf1c2e4..a1f9be7030021 100644
--- a/kernel/tracepoint.c
+++ b/kernel/tracepoint.c
@@ -59,6 +59,12 @@ struct tp_probes {
struct tracepoint_func probes[0];
 };
 
+/* Called in removal of a func but failed to allocate a new tp_funcs */
+static void tp_stub_func(void)
+{
+   return;
+}
+
 static inline void *allocate_probes(int count)
 {
struct tp_probes *p  = kmalloc(count * sizeof(struct tracepoint_func)
@@ -97,6 +103,7 @@ func_add(struct tracepoint_func **funcs, struct 
tracepoint_func *tp_func,
 {
struct tracepoint_func *old, *new;
int nr_probes = 0;
+   int stub_funcs = 0;
int pos = -1;
 
if (WARN_ON(!tp_func->func))
@@ -113,14 +120,34 @@ func_add(struct tracepoint_func **funcs, struct 
tracepoint_func *tp_func,
if (old[nr_probes].func == tp_func->func &&
old[nr_probes].data == tp_func->data)
return ERR_PTR(-EEXIST);
+   if (old[nr_probes].func == tp_stub_func)
+   stub_funcs++;
}
}
-   /* + 2 : one for new probe, one for NULL func */
-   new = allocate_probes(nr_probes + 2);
+   /* + 2 : one for new probe, one for NULL func - stub functions */
+   new = allocate_probes(nr_probes + 2 - stub_funcs);
if (new == NULL)
return ERR_PTR(-ENOMEM);
if (old) {
-   if (pos < 0) {
+   if (stub_funcs) {
+   /* Need to copy one at a time to remove stubs */
+   int probes = 0;
+
+   pos = -1;
+   for (nr_probes = 0; old[nr_probes].func; nr_probes++) {
+   if (old[nr_probes].func == tp_stub_func)
+   continue;
+   if (pos < 0 && old[nr_probes].prio < prio)
+   pos = probes++;
+   new[probes++] = old[nr_probes];
+   }
+   nr_probes = probes;
+   if (pos < 0)
+   pos = probes;
+   else
+   nr_probes--; /* Account for insertion */
+
+   } else if (pos < 0) {
pos = nr_probes;

Re: [PATCH mlx5-next v7 1/4] PCI: Add a sysfs file to change the MSI-X table size of SR-IOV VFs

[Greg Kroah-Hartman]

On Mon, Mar 01, 2021 at 10:32:09AM +0200, Leon Romanovsky wrote:
> On Mon, Mar 01, 2021 at 09:14:42AM +0100, Greg Kroah-Hartman wrote:
> > On Mon, Mar 01, 2021 at 09:55:21AM +0200, Leon Romanovsky wrote:
> > > From: Leon Romanovsky 
> > >
> > > A typical cloud provider SR-IOV use case is to create many VFs for use by
> > > guest VMs. The VFs may not be assigned to a VM until a customer requests a
> > > VM of a certain size, e.g., number of CPUs. A VF may need MSI-X vectors
> > > proportional to the number of CPUs in the VM, but there is no standard way
> > > to change the number of MSI-X vectors supported by a VF.
> > >
> > > Some Mellanox ConnectX devices support dynamic assignment of MSI-X vectors
> > > to SR-IOV VFs. This can be done by the PF driver after VFs are enabled,
> > > and it can be done without affecting VFs that are already in use. The
> > > hardware supports a limited pool of MSI-X vectors that can be assigned to
> > > the PF or to individual VFs.  This is device-specific behavior that
> > > requires support in the PF driver.
> > >
> > > Add a read-only "sriov_vf_total_msix" sysfs file for the PF and a writable
> > > "sriov_vf_msix_count" file for each VF. Management software may use these
> > > to learn how many MSI-X vectors are available and to dynamically assign
> > > them to VFs before the VFs are passed through to a VM.
> > >
> > > If the PF driver implements the ->sriov_get_vf_total_msix() callback,
> > > "sriov_vf_total_msix" contains the total number of MSI-X vectors available
> > > for distribution among VFs.
> > >
> > > If no driver is bound to the VF, writing "N" to "sriov_vf_msix_count" uses
> > > the PF driver ->sriov_set_msix_vec_count() callback to assign "N" MSI-X
> > > vectors to the VF.  When a VF driver subsequently reads the MSI-X Message
> > > Control register, it will see the new Table Size "N".
> > >
> > > Signed-off-by: Leon Romanovsky 
> > > ---
> > >  Documentation/ABI/testing/sysfs-bus-pci |  29 +++
> > >  drivers/pci/iov.c   | 102 ++--
> > >  drivers/pci/pci-sysfs.c |   3 +-
> > >  drivers/pci/pci.h   |   3 +-
> > >  include/linux/pci.h |   8 ++
> > >  5 files changed, 137 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/Documentation/ABI/testing/sysfs-bus-pci 
> > > b/Documentation/ABI/testing/sysfs-bus-pci
> > > index 25c9c39770c6..ebabd0d2ae88 100644
> > > --- a/Documentation/ABI/testing/sysfs-bus-pci
> > > +++ b/Documentation/ABI/testing/sysfs-bus-pci
> > > @@ -375,3 +375,32 @@ Description:
> > >   The value comes from the PCI kernel device state and can be one
> > >   of: "unknown", "error", "D0", D1", "D2", "D3hot", "D3cold".
> > >   The file is read only.
> > > +
> > > +What:/sys/bus/pci/devices/.../sriov_vf_total_msix
> > > +Date:January 2021
> > > +Contact: Leon Romanovsky 
> > > +Description:
> > > + This file is associated with a SR-IOV PF.  It contains the
> > > + total number of MSI-X vectors available for assignment to
> > > + all VFs associated with PF.  It will zero if the device
> >
> > "The value will be zero if the device..."
> 
> Thanks, will fix when apply or resend if more fixes will be needed.
> 
> >
> > And definition of "VF" and PF" are where in this file?
> 
> They come from the PCI spec. It is part of SR-IOV lingo.

Yes, and the world does not have access to the PCI spec, so please
provide a hint as to what they are for those of us without access to
such things.

thanks,

greg k-h

Re: [PATCH mlx5-next v7 1/4] PCI: Add a sysfs file to change the MSI-X table size of SR-IOV VFs

[Greg Kroah-Hartman]

On Mon, Mar 01, 2021 at 09:55:21AM +0200, Leon Romanovsky wrote:
> From: Leon Romanovsky 
> 
> A typical cloud provider SR-IOV use case is to create many VFs for use by
> guest VMs. The VFs may not be assigned to a VM until a customer requests a
> VM of a certain size, e.g., number of CPUs. A VF may need MSI-X vectors
> proportional to the number of CPUs in the VM, but there is no standard way
> to change the number of MSI-X vectors supported by a VF.
> 
> Some Mellanox ConnectX devices support dynamic assignment of MSI-X vectors
> to SR-IOV VFs. This can be done by the PF driver after VFs are enabled,
> and it can be done without affecting VFs that are already in use. The
> hardware supports a limited pool of MSI-X vectors that can be assigned to
> the PF or to individual VFs.  This is device-specific behavior that
> requires support in the PF driver.
> 
> Add a read-only "sriov_vf_total_msix" sysfs file for the PF and a writable
> "sriov_vf_msix_count" file for each VF. Management software may use these
> to learn how many MSI-X vectors are available and to dynamically assign
> them to VFs before the VFs are passed through to a VM.
> 
> If the PF driver implements the ->sriov_get_vf_total_msix() callback,
> "sriov_vf_total_msix" contains the total number of MSI-X vectors available
> for distribution among VFs.
> 
> If no driver is bound to the VF, writing "N" to "sriov_vf_msix_count" uses
> the PF driver ->sriov_set_msix_vec_count() callback to assign "N" MSI-X
> vectors to the VF.  When a VF driver subsequently reads the MSI-X Message
> Control register, it will see the new Table Size "N".
> 
> Signed-off-by: Leon Romanovsky 
> ---
>  Documentation/ABI/testing/sysfs-bus-pci |  29 +++
>  drivers/pci/iov.c   | 102 ++--
>  drivers/pci/pci-sysfs.c |   3 +-
>  drivers/pci/pci.h   |   3 +-
>  include/linux/pci.h |   8 ++
>  5 files changed, 137 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-pci 
> b/Documentation/ABI/testing/sysfs-bus-pci
> index 25c9c39770c6..ebabd0d2ae88 100644
> --- a/Documentation/ABI/testing/sysfs-bus-pci
> +++ b/Documentation/ABI/testing/sysfs-bus-pci
> @@ -375,3 +375,32 @@ Description:
>   The value comes from the PCI kernel device state and can be one
>   of: "unknown", "error", "D0", D1", "D2", "D3hot", "D3cold".
>   The file is read only.
> +
> +What:/sys/bus/pci/devices/.../sriov_vf_total_msix
> +Date:January 2021
> +Contact: Leon Romanovsky 
> +Description:
> + This file is associated with a SR-IOV PF.  It contains the
> + total number of MSI-X vectors available for assignment to
> + all VFs associated with PF.  It will zero if the device

"The value will be zero if the device..."

And definition of "VF" and PF" are where in this file?

Otherwise looks semi-sane to me, thanks.

greg k-h

[PATCH] e1000e: use proper #include guard name in hw.h

[Greg Kroah-Hartman]

The include guard for the e1000e and e1000 hw.h files are the same, so
add the proper "E" term to the hw.h file for the e1000e driver.

This resolves some static analyzer warnings, like the one found by the
"lgtm.com" tool.

Cc: Jesse Brandeburg 
Cc: Tony Nguyen 
Cc: "David S. Miller" 
Cc: Jakub Kicinski 
Cc: intel-wired-...@lists.osuosl.org
Signed-off-by: Greg Kroah-Hartman 
---
 drivers/net/ethernet/intel/e1000e/hw.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/hw.h 
b/drivers/net/ethernet/intel/e1000e/hw.h
index 69a2329ea463..f7954cadd979 100644
--- a/drivers/net/ethernet/intel/e1000e/hw.h
+++ b/drivers/net/ethernet/intel/e1000e/hw.h
@@ -1,8 +1,8 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 /* Copyright(c) 1999 - 2018 Intel Corporation. */
 
-#ifndef _E1000_HW_H_
-#define _E1000_HW_H_
+#ifndef _E1000E_HW_H_
+#define _E1000E_HW_H_
 
 #include "regs.h"
 #include "defines.h"
-- 
2.30.1

Re: [PATCH mlx5-next v6 1/4] PCI: Add sysfs callback to allow MSI-X table size change of SR-IOV VFs

[Greg Kroah-Hartman]

On Tue, Feb 23, 2021 at 03:07:43PM -0600, Bjorn Helgaas wrote:
> On Sun, Feb 21, 2021 at 08:59:18AM +0200, Leon Romanovsky wrote:
> > On Sat, Feb 20, 2021 at 01:06:00PM -0600, Bjorn Helgaas wrote:
> > > On Fri, Feb 19, 2021 at 09:20:18AM +0100, Greg Kroah-Hartman wrote:
> > >
> > > > Ok, can you step back and try to explain what problem you are trying to
> > > > solve first, before getting bogged down in odd details?  I find it
> > > > highly unlikely that this is something "unique", but I could be wrong as
> > > > I do not understand what you are wanting to do here at all.
> > >
> > > We want to add two new sysfs files:
> > >
> > >   sriov_vf_total_msix, for PF devices
> > >   sriov_vf_msix_count, for VF devices associated with the PF
> > >
> > > AFAICT it is *acceptable* if they are both present always.  But it
> > > would be *ideal* if they were only present when a driver that
> > > implements the ->sriov_get_vf_total_msix() callback is bound to the
> > > PF.
> > 
> > BTW, we already have all possible combinations: static, static with
> > folder, with and without "sriov_" prefix, dynamic with and without
> > folders on VFs.
> > 
> > I need to know on which version I'll get Acked-by and that version I
> > will resubmit.
> 
> I propose that you make static attributes for both files, so
> "sriov_vf_total_msix" is visible for *every* PF in the system and
> "sriov_vf_msix_count" is visible for *every* VF in the system.
> 
> The PF "sriov_vf_total_msix" show function can return zero if there's
> no PF driver or it doesn't support ->sriov_get_vf_total_msix().
> (Incidentally, I think the documentation should mention that when it
> *is* supported, the contents of this file are *constant*, i.e., it
> does not decrease as vectors are assigned to VFs.)
> 
> The "sriov_vf_msix_count" set function can ignore writes if there's no
> PF driver or it doesn't support ->sriov_get_vf_total_msix(), or if a
> VF driver is bound.
> 
> Any userspace software must be able to deal with those scenarios
> anyway, so I don't think the mere presence or absence of the files is
> a meaningful signal to that software.

Hopefully, good luck with that!

> If we figure out a way to make the files visible only when the
> appropriate driver is bound, that might be nice and could always be
> done later.  But I don't think it's essential.

That seems reasonable, feel free to cc: me on the next patch series and
I'll try to review it, which should make more sense to me than this
email thread :)

thanks,

greg k-h

Re: [PATCH mlx5-next v6 1/4] PCI: Add sysfs callback to allow MSI-X table size change of SR-IOV VFs

[Greg Kroah-Hartman]

On Sun, Feb 21, 2021 at 03:55:18PM +0200, Leon Romanovsky wrote:
> On Sun, Feb 21, 2021 at 02:00:41PM +0100, Greg Kroah-Hartman wrote:
> > On Sat, Feb 20, 2021 at 01:06:00PM -0600, Bjorn Helgaas wrote:
> > > On Fri, Feb 19, 2021 at 09:20:18AM +0100, Greg Kroah-Hartman wrote:
> > >
> > > > Ok, can you step back and try to explain what problem you are trying to
> > > > solve first, before getting bogged down in odd details?  I find it
> > > > highly unlikely that this is something "unique", but I could be wrong as
> > > > I do not understand what you are wanting to do here at all.
> > >
> > > We want to add two new sysfs files:
> > >
> > >   sriov_vf_total_msix, for PF devices
> > >   sriov_vf_msix_count, for VF devices associated with the PF
> > >
> > > AFAICT it is *acceptable* if they are both present always.  But it
> > > would be *ideal* if they were only present when a driver that
> > > implements the ->sriov_get_vf_total_msix() callback is bound to the
> > > PF.
> >
> > Ok, so in the pci bus probe function, if the driver that successfully
> > binds to the device is of this type, then create the sysfs files.
> >
> > The driver core will properly emit a KOBJ_BIND message when the driver
> > is bound to the device, so userspace knows it is now safe to rescan the
> > device to see any new attributes.
> >
> > Here's some horrible pseudo-patch for where this probably should be
> > done:
> >
> >
> > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> > index ec44a79e951a..5a854a5e3977 100644
> > --- a/drivers/pci/pci-driver.c
> > +++ b/drivers/pci/pci-driver.c
> > @@ -307,8 +307,14 @@ static long local_pci_probe(void *_ddi)
> > pm_runtime_get_sync(dev);
> > pci_dev->driver = pci_drv;
> > rc = pci_drv->probe(pci_dev, ddi->id);
> > -   if (!rc)
> > +   if (!rc) {
> > +   /* If PF or FV driver was bound, let's add some more sysfs 
> > files */
> > +   if (pci_drv->is_pf)
> > +   device_add_groups(pci_dev->dev, pf_groups);
> > +   if (pci_drv->is_fv)
> > +   device_add_groups(pci_dev->dev, fv_groups);
> > return rc;
> > +   }
> > if (rc < 0) {
> > pci_dev->driver = NULL;
> > pm_runtime_put_sync(dev);
> >
> >
> >
> >
> > Add some proper error handling if device_add_groups() fails, and then do
> > the same thing to remove the sysfs files when the device is unbound from
> > the driver, and you should be good to go.
> >
> > Or is this what you all are talking about already and I'm just totally
> > confused?
> 
> There are two different things here. First we need to add sysfs files
> for VF as the event of PF driver bind, not for the VF binds.
> 
> In your pseudo code, it will look:
>   rc = pci_drv->probe(pci_dev, ddi->id);
>  -if (!rc)
>  +if (!rc) {
>  +/* If PF or FV driver was bound, let's add some more sysfs 
> files */
>  +if (pci_drv->is_pf) {
>  +  int i = 0;
>  +device_add_groups(pci_dev->dev, pf_groups);
>  +  for (i; i < pci_dev->totalVF; i++) {
>  +  struct pci_device vf_dev = 
> find_vf_device(pci_dev, i);
>  +
>  +device_add_groups(vf_dev->dev, fv_groups);

Hahaha, no.

You are randomly adding new sysfs files to a _DIFFERENT_ device than the
one that is currently undergoing the probe() call?  That's crazy.  And
will break userspace.

Why would you want that?  The device should ONLY change when the device
that controls it has a driver bound/unbound to it, that should NEVER
cause random other devices on the bus to change state or sysfs files.

>  +  }
>  +  }
>   return rc;
> 
> Second, the code proposed by me does that but with driver callback that
> PF calls during init/uninit.

That works too, but really, why not just have the pci core do it for
you?  That way you do not have to go and modify each and every PCI
driver to get this type of support.  PCI core things belong in the PCI
core, not in each individual driver.

thanks,

greg k-h

Re: [PATCH mlx5-next v6 1/4] PCI: Add sysfs callback to allow MSI-X table size change of SR-IOV VFs

[Greg Kroah-Hartman]

On Sat, Feb 20, 2021 at 01:06:00PM -0600, Bjorn Helgaas wrote:
> On Fri, Feb 19, 2021 at 09:20:18AM +0100, Greg Kroah-Hartman wrote:
> 
> > Ok, can you step back and try to explain what problem you are trying to
> > solve first, before getting bogged down in odd details?  I find it
> > highly unlikely that this is something "unique", but I could be wrong as
> > I do not understand what you are wanting to do here at all.
> 
> We want to add two new sysfs files:
> 
>   sriov_vf_total_msix, for PF devices
>   sriov_vf_msix_count, for VF devices associated with the PF
> 
> AFAICT it is *acceptable* if they are both present always.  But it
> would be *ideal* if they were only present when a driver that
> implements the ->sriov_get_vf_total_msix() callback is bound to the
> PF.

Ok, so in the pci bus probe function, if the driver that successfully
binds to the device is of this type, then create the sysfs files.

The driver core will properly emit a KOBJ_BIND message when the driver
is bound to the device, so userspace knows it is now safe to rescan the
device to see any new attributes.

Here's some horrible pseudo-patch for where this probably should be
done:


diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index ec44a79e951a..5a854a5e3977 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -307,8 +307,14 @@ static long local_pci_probe(void *_ddi)
pm_runtime_get_sync(dev);
pci_dev->driver = pci_drv;
rc = pci_drv->probe(pci_dev, ddi->id);
-   if (!rc)
+   if (!rc) {
+   /* If PF or FV driver was bound, let's add some more sysfs 
files */
+   if (pci_drv->is_pf)
+   device_add_groups(pci_dev->dev, pf_groups);
+   if (pci_drv->is_fv)
+   device_add_groups(pci_dev->dev, fv_groups);
return rc;
+   }
if (rc < 0) {
pci_dev->driver = NULL;
pm_runtime_put_sync(dev);




Add some proper error handling if device_add_groups() fails, and then do
the same thing to remove the sysfs files when the device is unbound from
the driver, and you should be good to go.

Or is this what you all are talking about already and I'm just totally
confused?

thanks,

greg k-h

Re: [PATCH mlx5-next v6 1/4] PCI: Add sysfs callback to allow MSI-X table size change of SR-IOV VFs

[Greg Kroah-Hartman]

On Fri, Feb 19, 2021 at 09:52:12AM +0200, Leon Romanovsky wrote:
> On Thu, Feb 18, 2021 at 04:39:50PM -0600, Bjorn Helgaas wrote:
> > On Thu, Feb 18, 2021 at 12:15:51PM +0200, Leon Romanovsky wrote:
> > > On Wed, Feb 17, 2021 at 12:02:39PM -0600, Bjorn Helgaas wrote:
> > > > [+cc Greg in case he wants to chime in on the sysfs discussion.
> > > > TL;DR: we're trying to add/remove sysfs files when a PCI driver that
> > > > supports certain callbacks binds or unbinds; series at
> > > > https://lore.kernel.org/r/20210209133445.700225-1-l...@kernel.org]
> > > >
> > > > On Tue, Feb 16, 2021 at 09:58:25PM +0200, Leon Romanovsky wrote:
> > > > > On Tue, Feb 16, 2021 at 10:12:12AM -0600, Bjorn Helgaas wrote:
> > > > > > On Tue, Feb 16, 2021 at 09:33:44AM +0200, Leon Romanovsky wrote:
> > > > > > > On Mon, Feb 15, 2021 at 03:01:06PM -0600, Bjorn Helgaas wrote:
> > > > > > > > On Tue, Feb 09, 2021 at 03:34:42PM +0200, Leon Romanovsky wrote:
> > > > > > > > > From: Leon Romanovsky 
> > > >
> > > > > > > > > +int pci_enable_vf_overlay(struct pci_dev *dev)
> > > > > > > > > +{
> > > > > > > > > + struct pci_dev *virtfn;
> > > > > > > > > + int id, ret;
> > > > > > > > > +
> > > > > > > > > + if (!dev->is_physfn || !dev->sriov->num_VFs)
> > > > > > > > > + return 0;
> > > > > > > > > +
> > > > > > > > > + ret = sysfs_create_files(&dev->dev.kobj, 
> > > > > > > > > sriov_pf_dev_attrs);
> > > > > > > >
> > > > > > > > But I still don't like the fact that we're calling
> > > > > > > > sysfs_create_files() and sysfs_remove_files() directly.  It 
> > > > > > > > makes
> > > > > > > > complication and opportunities for errors.
> > > > > > >
> > > > > > > It is not different from any other code that we have in the 
> > > > > > > kernel.
> > > > > >
> > > > > > It *is* different.  There is a general rule that drivers should not
> > > > > > call sysfs_* [1].  The PCI core is arguably not a "driver," but it 
> > > > > > is
> > > > > > still true that callers of sysfs_create_files() are very special, 
> > > > > > and
> > > > > > I'd prefer not to add another one.
> > > > >
> > > > > PCI for me is a bus, and bus is the right place to manage sysfs.
> > > > > But it doesn't matter, we understand each other positions.
> > > > >
> > > > > > > Let's be concrete, can you point to the errors in this code that I
> > > > > > > should fix?
> > > > > >
> > > > > > I'm not saying there are current errors; I'm saying the additional
> > > > > > code makes errors possible in future code.  For example, we hope 
> > > > > > that
> > > > > > other drivers can use these sysfs interfaces, and it's possible they
> > > > > > may not call pci_enable_vf_overlay() or pci_disable_vfs_overlay()
> > > > > > correctly.
> > > > >
> > > > > If not, we will fix, we just need is to ensure that sysfs name won't
> > > > > change, everything else is easy to change.
> > > > >
> > > > > > Or there may be races in device addition/removal.  We have current
> > > > > > issues in this area, e.g., [2], and they're fairly subtle.  I'm not
> > > > > > saying your patches have these issues; only that extra code makes 
> > > > > > more
> > > > > > chances for mistakes and it's more work to validate it.
> > > > > >
> > > > > > > > I don't see the advantage of creating these files only when
> > > > > > > > the PF driver supports this.  The management tools have to
> > > > > > > > deal with sriov_vf_total_msix == 0 and sriov_vf_msix_count ==
> > > > > > > > 0 anyway.  Having the sysfs files not be present at all might
> > > > > > > > be slightly prettier to the person running "ls", but I'm not
> > > > > > > > sure the code complication is worth that.
> > > > > > >
> > > > > > > It is more than "ls", right now sriov_numvfs is visible without
> > > > > > > relation to the driver, even if driver doesn't implement
> > > > > > > ".sriov_configure", which IMHO bad. We didn't want to repeat.
> > > > > > >
> > > > > > > Right now, we have many devices that supports SR-IOV, but small
> > > > > > > amount of them are capable to rewrite their VF MSI-X table siz.
> > > > > > > We don't want "to punish" and clatter their sysfs.
> > > > > >
> > > > > > I agree, it's clutter, but at least it's just cosmetic clutter
> > > > > > (but I'm willing to hear discussion about why it's more than
> > > > > > cosmetic; see below).
> > > > >
> > > > > It is more than cosmetic and IMHO it is related to the driver role.
> > > > > This feature is advertised, managed and configured by PF. It is very
> > > > > natural request that the PF will view/hide those sysfs files.
> > > >
> > > > Agreed, it's natural if the PF driver adds/removes those files.  But I
> > > > don't think it's *essential*, and they *could* be static because of
> > > > this:
> > > >
> > > > > > From the management software point of view, I don't think it 
> > > > > > matters.
> > > > > > That software already needs to deal with files that don't exist (on
> > > > > > old kernels) and files that contain zero (feature 

Re: [PATCH v4] staging: qlge: fix comment style in qlge_main.c

[Greg Kroah-Hartman]

On Tue, Feb 16, 2021 at 05:40:12PM +0800, Du Cheng wrote:
> align * in block comments on each line

That says _what_ you did, not _why_ you did it.

And "each line"?  You only did this once.

> This series of patches is for Task 10 of the Eudyptula Challenge

This isn't a "series" of patches, it is a single patch.  And no one
cares about why you are doing this, this text isn't needed at all.

thanks,

greg k-h

Re: [PATCH v3] staging: fix coding style in driver/staging/qlge/qlge_main.c

[Greg Kroah-Hartman]

On Tue, Feb 16, 2021 at 04:53:26PM +0800, Du Cheng wrote:
> align * in block comments on each line
> 
> changes v3:
> - add SUBSYSTEM in subject line
> - add explanation to past version of this patch
> 
> changes v2:
> - move closing of comment to the same line
> 
> changes v1:
> - align * in block comments

These "changes" should all go below the --- line, right?

And the subject shoudl be a bit more specific, look at how other commits
are done for this driver.  Something like:
Subject: staging: qlge: fix comment style in qlge_main.c
matches much better, and conveys what is actually happening here, right?

v4 please?

thanks,

greg k-h

Re: [PATCH v2] fix coding style in driver/staging/qlge/qlge_main.c

[Greg Kroah-Hartman]

On Tue, Feb 16, 2021 at 03:35:26PM +0800, Du Cheng wrote:
> align * in block comments on each line
> 
> Signed-off-by: Du Cheng 
> ---
>  drivers/staging/qlge/qlge_main.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/qlge/qlge_main.c 
> b/drivers/staging/qlge/qlge_main.c
> index 5516be3af898..2682a0e474bd 100644
> --- a/drivers/staging/qlge/qlge_main.c
> +++ b/drivers/staging/qlge/qlge_main.c
> @@ -3815,8 +3815,7 @@ static int qlge_adapter_down(struct qlge_adapter *qdev)
>  
>   qlge_tx_ring_clean(qdev);
>  
> - /* Call netif_napi_del() from common point.
> - */
> + /* Call netif_napi_del() from common point. */
>   for (i = 0; i < qdev->rss_ring_count; i++)
>   netif_napi_del(&qdev->rx_ring[i].napi);
>  
> -- 
> 2.27.0
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- You did not write a descriptive Subject: for the patch, allowing Greg,
  and everyone else, to know what this patch is all about.  Please read
  the section entitled "The canonical patch format" in the kernel file,
  Documentation/SubmittingPatches for what a proper Subject: line should
  look like.

- This looks like a new version of a previously submitted patch, but you
  did not list below the --- line any changes from the previous version.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/SubmittingPatches for what needs to be done
  here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

Re: [PATCH] fix coding style in driver/staging/qlge/qlge_main.c

[Greg Kroah-Hartman]

On Tue, Feb 16, 2021 at 03:18:49PM +0800, Du Cheng wrote:
> align * in block comments on each line
> 
> Signed-off-by: Du Cheng 
> ---
>  drivers/staging/qlge/qlge_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/qlge/qlge_main.c 
> b/drivers/staging/qlge/qlge_main.c
> index 5516be3af898..bfd7217f3953 100644
> --- a/drivers/staging/qlge/qlge_main.c
> +++ b/drivers/staging/qlge/qlge_main.c
> @@ -3816,7 +3816,7 @@ static int qlge_adapter_down(struct qlge_adapter *qdev)
>   qlge_tx_ring_clean(qdev);
>  
>   /* Call netif_napi_del() from common point.
> - */
> +  */

Just put it on the previous line please.

Also use scripts/get_maintainer.pl to find the proper lists to send this
to in the future, you forgot the staging mailing list :(

thanks,

greg k-h

Re: [PATCH v2 0/2] of: of_device.h cleanups

[Greg Kroah-Hartman]

On Thu, Feb 11, 2021 at 05:27:43PM -0600, Rob Herring wrote:
> This is a couple of cleanups for of_device.h. They fell out from my
> attempt at decoupling of_device.h and of_platform.h which is a mess
> and I haven't finished, but there's no reason to wait on these.

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH v2 1/2] mei: bus: simplify mei_cl_device_remove()

[Greg Kroah-Hartman]

On Mon, Feb 08, 2021 at 08:37:04AM +0100, Uwe Kleine-König wrote:
> The driver core only calls a bus' remove function when there is actually
> a driver and a device. So drop the needless check and assign cldrv earlier.
> 
> (Side note: The check for cldev being non-NULL is broken anyhow, because
> to_mei_cl_device() is a wrapper around container_of() for a member that is
> not the first one. So cldev only can become NULL if dev is (void *)0xc
> (for archs with 32 bit pointers) or (void *)0x18 (for archs with 64 bit
> pointers).)
> 
> Signed-off-by: Uwe Kleine-König 
> ---
>  drivers/misc/mei/bus.c | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/misc/mei/bus.c b/drivers/misc/mei/bus.c
> index 2907db260fba..50d617e7467e 100644
> --- a/drivers/misc/mei/bus.c
> +++ b/drivers/misc/mei/bus.c
> @@ -878,13 +878,9 @@ static int mei_cl_device_probe(struct device *dev)
>  static int mei_cl_device_remove(struct device *dev)
>  {
>   struct mei_cl_device *cldev = to_mei_cl_device(dev);
> - struct mei_cl_driver *cldrv;
> + struct mei_cl_driver *cldrv = to_mei_cl_driver(dev->driver);
>   int ret = 0;
>  
> - if (!cldev || !dev->driver)

Yes, anyone checking that the results of a container_of() wrapper can be
NULL is not checking anything at all :)

thanks for the cleanups, I'll go queue them up.

greg k-h

Re: [PATCH] vio: make remove callback return void

[Greg Kroah-Hartman]

On Wed, Jan 27, 2021 at 10:50:10PM +0100, Uwe Kleine-König wrote:
> The driver core ignores the return value of struct bus_type::remove()
> because there is only little that can be done. To simplify the quest to
> make this function return void, let struct vio_driver::remove() return
> void, too. All users already unconditionally return 0, this commit makes
> it obvious that returning an error code is a bad idea and makes it
> obvious for future driver authors that returning an error code isn't
> intended.
> 
> Note there are two nominally different implementations for a vio bus:
> one in arch/sparc/kernel/vio.c and the other in
> arch/powerpc/platforms/pseries/vio.c. I didn't care to check which
> driver is using which of these busses (or if even some of them can be
> used with both) and simply adapt all drivers and the two bus codes in
> one go.
> 
> Note that for the powerpc implementation there is a semantical change:
> Before this patch for a device that was bound to a driver without a
> remove callback vio_cmo_bus_remove(viodev) wasn't called. As the device
> core still considers the device unbound after vio_bus_remove() returns
> calling this unconditionally is the consistent behaviour which is
> implemented here.
> 
> Signed-off-by: Uwe Kleine-König 
> ---
> Hello,
> 
> note that this change depends on
> https://lore.kernel.org/r/20210121062005.53271-1-...@linux.ibm.com which 
> removes
> an (ignored) return -EBUSY in drivers/net/ethernet/ibm/ibmvnic.c.
> I don't know when/if this latter patch will be applied, so it might take
> some time until my patch can go in.

Acked-by: Greg Kroah-Hartman 

Re: [PATCH] staging: rtl8723bs: fix wireless regulatory API misuse

[Greg Kroah-Hartman]

On Tue, Jan 26, 2021 at 11:54:09AM +0100, Johannes Berg wrote:
> From: Johannes Berg 
> 
> This code ends up calling wiphy_apply_custom_regulatory(), for which
> we document that it should be called before wiphy_register(). This
> driver doesn't do that, but calls it from ndo_open() with the RTNL
> held, which caused deadlocks.
> 
> Since the driver just registers static regdomain data and then the
> notifier applies the channel changes if any, there's no reason for
> it to call this in ndo_open(), move it earlier to fix the deadlock.
> 
> Reported-and-tested-by: Hans de Goede 
> Fixes: 51d62f2f2c50 ("cfg80211: Save the regulatory domain with a lock")
> Signed-off-by: Johannes Berg 
> ---
> Greg, can you take this for 5.11 please? Or if you prefer, since the
> patch that exposed this and broke the driver went through my tree, I
> can take it as well.

Please feel free to take it through yours, as I don't think I'll have
any more staging patches for 5.11-final (or none have been sent to me
yet), so this might be the fastest way in:

Acked-by: Greg Kroah-Hartman 

Re: [PATCH net-next 1/1] stmmac: intel: change all EHL/TGL to auto detect phy addr

[Greg Kroah-Hartman]

On Sat, Jan 16, 2021 at 04:59:14PM -0800, Jakub Kicinski wrote:
> On Sat, 16 Jan 2021 10:12:21 +0100 Jan Kiszka wrote:
> > On 06.11.20 10:43, Wong Vee Khee wrote:
> > > From: Voon Weifeng 
> > > 
> > > Set all EHL/TGL phy_addr to -1 so that the driver will automatically
> > > detect it at run-time by probing all the possible 32 addresses.
> > > 
> > > Signed-off-by: Voon Weifeng 
> > > Signed-off-by: Wong Vee Khee 
> > 
> > This fixes PHY detection on one of our EHL-based boards. Can this also
> > be applied to stable 5.10?
> 
> Sure.
> 
> Greg, we'd like to request a backport of the following commit to 5.10.
> 
> commit bff6f1db91e330d7fba56f815cdbc412c75fe163
> Author: Voon Weifeng 
> Date:   Fri Nov 6 17:43:41 2020 +0800
> 
> stmmac: intel: change all EHL/TGL to auto detect phy addr
> 
> Set all EHL/TGL phy_addr to -1 so that the driver will automatically
> detect it at run-time by probing all the possible 32 addresses.
> 
> Signed-off-by: Voon Weifeng 
> Signed-off-by: Wong Vee Khee 
> Link: 
> https://lore.kernel.org/r/20201106094341.4241-1-vee.khee.w...@intel.com
> Signed-off-by: Jakub Kicinski 
> 
> 
> It's relatively small, and Jan reports it makes his boards detect the
> PHY. The change went in via -next and into Linus's tree during the 5.11
> merge window.

Now queued up, thanks.

greg k-h

Re: [PATCH AUTOSEL 5.10 23/51] CDC-NCM: remove "connected" log message

[Greg Kroah-Hartman]

On Tue, Jan 12, 2021 at 07:55:05AM -0500, Sasha Levin wrote:
> From: Roland Dreier 
> 
> [ Upstream commit 59b4a8fa27f5a895582ada1ae5034af7c94a57b5 ]
> 
> The cdc_ncm driver passes network connection notifications up to
> usbnet_link_change(), which is the right place for any logging.
> Remove the netdev_info() duplicating this from the driver itself.
> 
> This stops devices such as my "TRENDnet USB 10/100/1G/2.5G LAN"
> (ID 20f4:e02b) adapter from spamming the kernel log with
> 
> cdc_ncm 2-2:2.0 enp0s2u2c2: network connection: connected
> 
> messages every 60 msec or so.
> 
> Signed-off-by: Roland Dreier 
> Reviewed-by: Greg Kroah-Hartman 
> Link: https://lore.kernel.org/r/20201224032116.2453938-1-rol...@kernel.org
> Signed-off-by: Jakub Kicinski 
> Signed-off-by: Sasha Levin 
> ---
>  drivers/net/usb/cdc_ncm.c | 3 ---
>  1 file changed, 3 deletions(-)

This is already queued up to be in the next round of stable releases, so
no need to queue it up again :)

thanks,

greg k-h

[PATCH 4.9 15/45] net: sched: prevent invalid Scell_log shift count

[Greg Kroah-Hartman]

From: Randy Dunlap 

[ Upstream commit bd1248f1ddbc48b0c30565fce897a3b6423313b8 ]

Check Scell_log shift size in red_check_params() and modify all callers
of red_check_params() to pass Scell_log.

This prevents a shift out-of-bounds as detected by UBSAN:
  UBSAN: shift-out-of-bounds in ./include/net/red.h:252:22
  shift exponent 72 is too large for 32-bit type 'int'

Fixes: 8afa10cbe281 ("net_sched: red: Avoid illegal values")
Signed-off-by: Randy Dunlap 
Reported-by: syzbot+97c5bd9cc81eca63d...@syzkaller.appspotmail.com
Cc: Nogah Frankel 
Cc: Jamal Hadi Salim 
Cc: Cong Wang 
Cc: Jiri Pirko 
Cc: netdev@vger.kernel.org
Cc: "David S. Miller" 
Cc: Jakub Kicinski 
Signed-off-by: David S. Miller 
Signed-off-by: Greg Kroah-Hartman 
---
 include/net/red.h |4 +++-
 net/sched/sch_choke.c |2 +-
 net/sched/sch_gred.c  |2 +-
 net/sched/sch_red.c   |2 +-
 net/sched/sch_sfq.c   |2 +-
 5 files changed, 7 insertions(+), 5 deletions(-)

--- a/include/net/red.h
+++ b/include/net/red.h
@@ -167,12 +167,14 @@ static inline void red_set_vars(struct r
v->qcount   = -1;
 }
 
-static inline bool red_check_params(u32 qth_min, u32 qth_max, u8 Wlog)
+static inline bool red_check_params(u32 qth_min, u32 qth_max, u8 Wlog, u8 
Scell_log)
 {
if (fls(qth_min) + Wlog > 32)
return false;
if (fls(qth_max) + Wlog > 32)
return false;
+   if (Scell_log >= 32)
+   return false;
if (qth_max < qth_min)
return false;
return true;
--- a/net/sched/sch_choke.c
+++ b/net/sched/sch_choke.c
@@ -425,7 +425,7 @@ static int choke_change(struct Qdisc *sc
 
ctl = nla_data(tb[TCA_CHOKE_PARMS]);
 
-   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog))
+   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog, 
ctl->Scell_log))
return -EINVAL;
 
if (ctl->limit > CHOKE_MAX_QUEUE)
--- a/net/sched/sch_gred.c
+++ b/net/sched/sch_gred.c
@@ -356,7 +356,7 @@ static inline int gred_change_vq(struct
struct gred_sched *table = qdisc_priv(sch);
struct gred_sched_data *q = table->tab[dp];
 
-   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog))
+   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog, 
ctl->Scell_log))
return -EINVAL;
 
if (!q) {
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -184,7 +184,7 @@ static int red_change(struct Qdisc *sch,
max_P = tb[TCA_RED_MAX_P] ? nla_get_u32(tb[TCA_RED_MAX_P]) : 0;
 
ctl = nla_data(tb[TCA_RED_PARMS]);
-   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog))
+   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog, 
ctl->Scell_log))
return -EINVAL;
 
if (ctl->limit > 0) {
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -645,7 +645,7 @@ static int sfq_change(struct Qdisc *sch,
}
 
if (ctl_v1 && !red_check_params(ctl_v1->qth_min, ctl_v1->qth_max,
-   ctl_v1->Wlog))
+   ctl_v1->Wlog, ctl_v1->Scell_log))
return -EINVAL;
if (ctl_v1 && ctl_v1->qth_min) {
p = kmalloc(sizeof(*p), GFP_KERNEL);

[PATCH 4.19 35/77] net: sched: prevent invalid Scell_log shift count

[Greg Kroah-Hartman]

From: Randy Dunlap 

[ Upstream commit bd1248f1ddbc48b0c30565fce897a3b6423313b8 ]

Check Scell_log shift size in red_check_params() and modify all callers
of red_check_params() to pass Scell_log.

This prevents a shift out-of-bounds as detected by UBSAN:
  UBSAN: shift-out-of-bounds in ./include/net/red.h:252:22
  shift exponent 72 is too large for 32-bit type 'int'

Fixes: 8afa10cbe281 ("net_sched: red: Avoid illegal values")
Signed-off-by: Randy Dunlap 
Reported-by: syzbot+97c5bd9cc81eca63d...@syzkaller.appspotmail.com
Cc: Nogah Frankel 
Cc: Jamal Hadi Salim 
Cc: Cong Wang 
Cc: Jiri Pirko 
Cc: netdev@vger.kernel.org
Cc: "David S. Miller" 
Cc: Jakub Kicinski 
Signed-off-by: David S. Miller 
Signed-off-by: Greg Kroah-Hartman 
---
 include/net/red.h |4 +++-
 net/sched/sch_choke.c |2 +-
 net/sched/sch_gred.c  |2 +-
 net/sched/sch_red.c   |2 +-
 net/sched/sch_sfq.c   |2 +-
 5 files changed, 7 insertions(+), 5 deletions(-)

--- a/include/net/red.h
+++ b/include/net/red.h
@@ -168,12 +168,14 @@ static inline void red_set_vars(struct r
v->qcount   = -1;
 }
 
-static inline bool red_check_params(u32 qth_min, u32 qth_max, u8 Wlog)
+static inline bool red_check_params(u32 qth_min, u32 qth_max, u8 Wlog, u8 
Scell_log)
 {
if (fls(qth_min) + Wlog > 32)
return false;
if (fls(qth_max) + Wlog > 32)
return false;
+   if (Scell_log >= 32)
+   return false;
if (qth_max < qth_min)
return false;
return true;
--- a/net/sched/sch_choke.c
+++ b/net/sched/sch_choke.c
@@ -371,7 +371,7 @@ static int choke_change(struct Qdisc *sc
 
ctl = nla_data(tb[TCA_CHOKE_PARMS]);
 
-   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog))
+   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog, 
ctl->Scell_log))
return -EINVAL;
 
if (ctl->limit > CHOKE_MAX_QUEUE)
--- a/net/sched/sch_gred.c
+++ b/net/sched/sch_gred.c
@@ -357,7 +357,7 @@ static inline int gred_change_vq(struct
struct gred_sched *table = qdisc_priv(sch);
struct gred_sched_data *q = table->tab[dp];
 
-   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog))
+   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog, 
ctl->Scell_log))
return -EINVAL;
 
if (!q) {
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -214,7 +214,7 @@ static int red_change(struct Qdisc *sch,
max_P = tb[TCA_RED_MAX_P] ? nla_get_u32(tb[TCA_RED_MAX_P]) : 0;
 
ctl = nla_data(tb[TCA_RED_PARMS]);
-   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog))
+   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog, 
ctl->Scell_log))
return -EINVAL;
 
if (ctl->limit > 0) {
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -651,7 +651,7 @@ static int sfq_change(struct Qdisc *sch,
}
 
if (ctl_v1 && !red_check_params(ctl_v1->qth_min, ctl_v1->qth_max,
-   ctl_v1->Wlog))
+   ctl_v1->Wlog, ctl_v1->Scell_log))
return -EINVAL;
if (ctl_v1 && ctl_v1->qth_min) {
p = kmalloc(sizeof(*p), GFP_KERNEL);

[PATCH 5.4 34/92] net: sched: prevent invalid Scell_log shift count

[Greg Kroah-Hartman]

From: Randy Dunlap 

[ Upstream commit bd1248f1ddbc48b0c30565fce897a3b6423313b8 ]

Check Scell_log shift size in red_check_params() and modify all callers
of red_check_params() to pass Scell_log.

This prevents a shift out-of-bounds as detected by UBSAN:
  UBSAN: shift-out-of-bounds in ./include/net/red.h:252:22
  shift exponent 72 is too large for 32-bit type 'int'

Fixes: 8afa10cbe281 ("net_sched: red: Avoid illegal values")
Signed-off-by: Randy Dunlap 
Reported-by: syzbot+97c5bd9cc81eca63d...@syzkaller.appspotmail.com
Cc: Nogah Frankel 
Cc: Jamal Hadi Salim 
Cc: Cong Wang 
Cc: Jiri Pirko 
Cc: netdev@vger.kernel.org
Cc: "David S. Miller" 
Cc: Jakub Kicinski 
Signed-off-by: David S. Miller 
Signed-off-by: Greg Kroah-Hartman 
---
 include/net/red.h |4 +++-
 net/sched/sch_choke.c |2 +-
 net/sched/sch_gred.c  |2 +-
 net/sched/sch_red.c   |2 +-
 net/sched/sch_sfq.c   |2 +-
 5 files changed, 7 insertions(+), 5 deletions(-)

--- a/include/net/red.h
+++ b/include/net/red.h
@@ -168,12 +168,14 @@ static inline void red_set_vars(struct r
v->qcount   = -1;
 }
 
-static inline bool red_check_params(u32 qth_min, u32 qth_max, u8 Wlog)
+static inline bool red_check_params(u32 qth_min, u32 qth_max, u8 Wlog, u8 
Scell_log)
 {
if (fls(qth_min) + Wlog > 32)
return false;
if (fls(qth_max) + Wlog > 32)
return false;
+   if (Scell_log >= 32)
+   return false;
if (qth_max < qth_min)
return false;
return true;
--- a/net/sched/sch_choke.c
+++ b/net/sched/sch_choke.c
@@ -368,7 +368,7 @@ static int choke_change(struct Qdisc *sc
 
ctl = nla_data(tb[TCA_CHOKE_PARMS]);
 
-   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog))
+   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog, 
ctl->Scell_log))
return -EINVAL;
 
if (ctl->limit > CHOKE_MAX_QUEUE)
--- a/net/sched/sch_gred.c
+++ b/net/sched/sch_gred.c
@@ -480,7 +480,7 @@ static inline int gred_change_vq(struct
struct gred_sched *table = qdisc_priv(sch);
struct gred_sched_data *q = table->tab[dp];
 
-   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog)) {
+   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog, 
ctl->Scell_log)) {
NL_SET_ERR_MSG_MOD(extack, "invalid RED parameters");
return -EINVAL;
}
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -213,7 +213,7 @@ static int red_change(struct Qdisc *sch,
max_P = tb[TCA_RED_MAX_P] ? nla_get_u32(tb[TCA_RED_MAX_P]) : 0;
 
ctl = nla_data(tb[TCA_RED_PARMS]);
-   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog))
+   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog, 
ctl->Scell_log))
return -EINVAL;
 
if (ctl->limit > 0) {
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -647,7 +647,7 @@ static int sfq_change(struct Qdisc *sch,
}
 
if (ctl_v1 && !red_check_params(ctl_v1->qth_min, ctl_v1->qth_max,
-   ctl_v1->Wlog))
+   ctl_v1->Wlog, ctl_v1->Scell_log))
return -EINVAL;
if (ctl_v1 && ctl_v1->qth_min) {
p = kmalloc(sizeof(*p), GFP_KERNEL);

[PATCH 5.10 035/145] net: sched: prevent invalid Scell_log shift count

[Greg Kroah-Hartman]

From: Randy Dunlap 

[ Upstream commit bd1248f1ddbc48b0c30565fce897a3b6423313b8 ]

Check Scell_log shift size in red_check_params() and modify all callers
of red_check_params() to pass Scell_log.

This prevents a shift out-of-bounds as detected by UBSAN:
  UBSAN: shift-out-of-bounds in ./include/net/red.h:252:22
  shift exponent 72 is too large for 32-bit type 'int'

Fixes: 8afa10cbe281 ("net_sched: red: Avoid illegal values")
Signed-off-by: Randy Dunlap 
Reported-by: syzbot+97c5bd9cc81eca63d...@syzkaller.appspotmail.com
Cc: Nogah Frankel 
Cc: Jamal Hadi Salim 
Cc: Cong Wang 
Cc: Jiri Pirko 
Cc: netdev@vger.kernel.org
Cc: "David S. Miller" 
Cc: Jakub Kicinski 
Signed-off-by: David S. Miller 
Signed-off-by: Greg Kroah-Hartman 
---
 include/net/red.h |4 +++-
 net/sched/sch_choke.c |2 +-
 net/sched/sch_gred.c  |2 +-
 net/sched/sch_red.c   |2 +-
 net/sched/sch_sfq.c   |2 +-
 5 files changed, 7 insertions(+), 5 deletions(-)

--- a/include/net/red.h
+++ b/include/net/red.h
@@ -168,12 +168,14 @@ static inline void red_set_vars(struct r
v->qcount   = -1;
 }
 
-static inline bool red_check_params(u32 qth_min, u32 qth_max, u8 Wlog)
+static inline bool red_check_params(u32 qth_min, u32 qth_max, u8 Wlog, u8 
Scell_log)
 {
if (fls(qth_min) + Wlog > 32)
return false;
if (fls(qth_max) + Wlog > 32)
return false;
+   if (Scell_log >= 32)
+   return false;
if (qth_max < qth_min)
return false;
return true;
--- a/net/sched/sch_choke.c
+++ b/net/sched/sch_choke.c
@@ -362,7 +362,7 @@ static int choke_change(struct Qdisc *sc
 
ctl = nla_data(tb[TCA_CHOKE_PARMS]);
 
-   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog))
+   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog, 
ctl->Scell_log))
return -EINVAL;
 
if (ctl->limit > CHOKE_MAX_QUEUE)
--- a/net/sched/sch_gred.c
+++ b/net/sched/sch_gred.c
@@ -480,7 +480,7 @@ static inline int gred_change_vq(struct
struct gred_sched *table = qdisc_priv(sch);
struct gred_sched_data *q = table->tab[dp];
 
-   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog)) {
+   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog, 
ctl->Scell_log)) {
NL_SET_ERR_MSG_MOD(extack, "invalid RED parameters");
return -EINVAL;
}
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -250,7 +250,7 @@ static int __red_change(struct Qdisc *sc
max_P = tb[TCA_RED_MAX_P] ? nla_get_u32(tb[TCA_RED_MAX_P]) : 0;
 
ctl = nla_data(tb[TCA_RED_PARMS]);
-   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog))
+   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog, 
ctl->Scell_log))
return -EINVAL;
 
err = red_get_flags(ctl->flags, TC_RED_HISTORIC_FLAGS,
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -647,7 +647,7 @@ static int sfq_change(struct Qdisc *sch,
}
 
if (ctl_v1 && !red_check_params(ctl_v1->qth_min, ctl_v1->qth_max,
-   ctl_v1->Wlog))
+   ctl_v1->Wlog, ctl_v1->Scell_log))
return -EINVAL;
if (ctl_v1 && ctl_v1->qth_min) {
p = kmalloc(sizeof(*p), GFP_KERNEL);

[PATCH 4.14 23/57] net: sched: prevent invalid Scell_log shift count

[Greg Kroah-Hartman]

From: Randy Dunlap 

[ Upstream commit bd1248f1ddbc48b0c30565fce897a3b6423313b8 ]

Check Scell_log shift size in red_check_params() and modify all callers
of red_check_params() to pass Scell_log.

This prevents a shift out-of-bounds as detected by UBSAN:
  UBSAN: shift-out-of-bounds in ./include/net/red.h:252:22
  shift exponent 72 is too large for 32-bit type 'int'

Fixes: 8afa10cbe281 ("net_sched: red: Avoid illegal values")
Signed-off-by: Randy Dunlap 
Reported-by: syzbot+97c5bd9cc81eca63d...@syzkaller.appspotmail.com
Cc: Nogah Frankel 
Cc: Jamal Hadi Salim 
Cc: Cong Wang 
Cc: Jiri Pirko 
Cc: netdev@vger.kernel.org
Cc: "David S. Miller" 
Cc: Jakub Kicinski 
Signed-off-by: David S. Miller 
Signed-off-by: Greg Kroah-Hartman 
---
 include/net/red.h |4 +++-
 net/sched/sch_choke.c |2 +-
 net/sched/sch_gred.c  |2 +-
 net/sched/sch_red.c   |2 +-
 net/sched/sch_sfq.c   |2 +-
 5 files changed, 7 insertions(+), 5 deletions(-)

--- a/include/net/red.h
+++ b/include/net/red.h
@@ -168,12 +168,14 @@ static inline void red_set_vars(struct r
v->qcount   = -1;
 }
 
-static inline bool red_check_params(u32 qth_min, u32 qth_max, u8 Wlog)
+static inline bool red_check_params(u32 qth_min, u32 qth_max, u8 Wlog, u8 
Scell_log)
 {
if (fls(qth_min) + Wlog > 32)
return false;
if (fls(qth_max) + Wlog > 32)
return false;
+   if (Scell_log >= 32)
+   return false;
if (qth_max < qth_min)
return false;
return true;
--- a/net/sched/sch_choke.c
+++ b/net/sched/sch_choke.c
@@ -370,7 +370,7 @@ static int choke_change(struct Qdisc *sc
 
ctl = nla_data(tb[TCA_CHOKE_PARMS]);
 
-   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog))
+   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog, 
ctl->Scell_log))
return -EINVAL;
 
if (ctl->limit > CHOKE_MAX_QUEUE)
--- a/net/sched/sch_gred.c
+++ b/net/sched/sch_gred.c
@@ -356,7 +356,7 @@ static inline int gred_change_vq(struct
struct gred_sched *table = qdisc_priv(sch);
struct gred_sched_data *q = table->tab[dp];
 
-   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog))
+   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog, 
ctl->Scell_log))
return -EINVAL;
 
if (!q) {
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -184,7 +184,7 @@ static int red_change(struct Qdisc *sch,
max_P = tb[TCA_RED_MAX_P] ? nla_get_u32(tb[TCA_RED_MAX_P]) : 0;
 
ctl = nla_data(tb[TCA_RED_PARMS]);
-   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog))
+   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog, 
ctl->Scell_log))
return -EINVAL;
 
if (ctl->limit > 0) {
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -649,7 +649,7 @@ static int sfq_change(struct Qdisc *sch,
}
 
if (ctl_v1 && !red_check_params(ctl_v1->qth_min, ctl_v1->qth_max,
-   ctl_v1->Wlog))
+   ctl_v1->Wlog, ctl_v1->Scell_log))
return -EINVAL;
if (ctl_v1 && ctl_v1->qth_min) {
p = kmalloc(sizeof(*p), GFP_KERNEL);

[PATCH 4.4 12/38] net: sched: prevent invalid Scell_log shift count

[Greg Kroah-Hartman]

From: Randy Dunlap 

[ Upstream commit bd1248f1ddbc48b0c30565fce897a3b6423313b8 ]

Check Scell_log shift size in red_check_params() and modify all callers
of red_check_params() to pass Scell_log.

This prevents a shift out-of-bounds as detected by UBSAN:
  UBSAN: shift-out-of-bounds in ./include/net/red.h:252:22
  shift exponent 72 is too large for 32-bit type 'int'

Fixes: 8afa10cbe281 ("net_sched: red: Avoid illegal values")
Signed-off-by: Randy Dunlap 
Reported-by: syzbot+97c5bd9cc81eca63d...@syzkaller.appspotmail.com
Cc: Nogah Frankel 
Cc: Jamal Hadi Salim 
Cc: Cong Wang 
Cc: Jiri Pirko 
Cc: netdev@vger.kernel.org
Cc: "David S. Miller" 
Cc: Jakub Kicinski 
Signed-off-by: David S. Miller 
Signed-off-by: Greg Kroah-Hartman 
---
 include/net/red.h |4 +++-
 net/sched/sch_choke.c |2 +-
 net/sched/sch_gred.c  |2 +-
 net/sched/sch_red.c   |2 +-
 net/sched/sch_sfq.c   |2 +-
 5 files changed, 7 insertions(+), 5 deletions(-)

--- a/include/net/red.h
+++ b/include/net/red.h
@@ -167,12 +167,14 @@ static inline void red_set_vars(struct r
v->qcount   = -1;
 }
 
-static inline bool red_check_params(u32 qth_min, u32 qth_max, u8 Wlog)
+static inline bool red_check_params(u32 qth_min, u32 qth_max, u8 Wlog, u8 
Scell_log)
 {
if (fls(qth_min) + Wlog > 32)
return false;
if (fls(qth_max) + Wlog > 32)
return false;
+   if (Scell_log >= 32)
+   return false;
if (qth_max < qth_min)
return false;
return true;
--- a/net/sched/sch_choke.c
+++ b/net/sched/sch_choke.c
@@ -439,7 +439,7 @@ static int choke_change(struct Qdisc *sc
 
ctl = nla_data(tb[TCA_CHOKE_PARMS]);
 
-   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog))
+   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog, 
ctl->Scell_log))
return -EINVAL;
 
if (ctl->limit > CHOKE_MAX_QUEUE)
--- a/net/sched/sch_gred.c
+++ b/net/sched/sch_gred.c
@@ -389,7 +389,7 @@ static inline int gred_change_vq(struct
struct gred_sched *table = qdisc_priv(sch);
struct gred_sched_data *q = table->tab[dp];
 
-   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog))
+   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog, 
ctl->Scell_log))
return -EINVAL;
 
if (!q) {
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -203,7 +203,7 @@ static int red_change(struct Qdisc *sch,
max_P = tb[TCA_RED_MAX_P] ? nla_get_u32(tb[TCA_RED_MAX_P]) : 0;
 
ctl = nla_data(tb[TCA_RED_PARMS]);
-   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog))
+   if (!red_check_params(ctl->qth_min, ctl->qth_max, ctl->Wlog, 
ctl->Scell_log))
return -EINVAL;
 
if (ctl->limit > 0) {
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -645,7 +645,7 @@ static int sfq_change(struct Qdisc *sch,
}
 
if (ctl_v1 && !red_check_params(ctl_v1->qth_min, ctl_v1->qth_max,
-   ctl_v1->Wlog))
+   ctl_v1->Wlog, ctl_v1->Scell_log))
return -EINVAL;
if (ctl_v1 && ctl_v1->qth_min) {
p = kmalloc(sizeof(*p), GFP_KERNEL);

Re: [PATCH] dt-bindings: Add missing array size constraints

[Greg Kroah-Hartman]

On Tue, Jan 05, 2021 at 10:40:08AM -0700, Rob Herring wrote:
> On Tue, Jan 05, 2021 at 02:04:14PM +0100, Greg Kroah-Hartman wrote:
> > On Mon, Jan 04, 2021 at 04:02:53PM -0700, Rob Herring wrote:
> > > DT properties which can have multiple entries need to specify what the
> > > entries are and define how many entries there can be. In the case of
> > > only a single entry, just 'maxItems: 1' is sufficient.
> > > 
> > > Add the missing entry constraints. These were found with a modified
> > > meta-schema. Unfortunately, there are a few cases where the size
> > > constraints are not defined such as common bindings, so the meta-schema
> > > can't be part of the normal checks.
> > > 
> > > Cc: Jens Axboe 
> > > Cc: Stephen Boyd 
> > > Cc: Thierry Reding 
> > > Cc: MyungJoo Ham 
> > > Cc: Chanwoo Choi 
> > > Cc: Linus Walleij 
> > > Cc: Bartosz Golaszewski 
> > > Cc: Jonathan Cameron 
> > > Cc: Dmitry Torokhov 
> > > Cc: Thomas Gleixner 
> > > Cc: Marc Zyngier 
> > > Cc: Mauro Carvalho Chehab 
> > > Cc: Chen-Yu Tsai 
> > > Cc: Ulf Hansson 
> > > Cc: "David S. Miller" 
> > > Cc: Jakub Kicinski 
> > > Cc: Sebastian Reichel 
> > > Cc: Ohad Ben-Cohen 
> > > Cc: Bjorn Andersson 
> > > Cc: Greg Kroah-Hartman 
> > > Signed-off-by: Rob Herring 
> > 
> > 
> > 
> > > diff --git a/Documentation/devicetree/bindings/usb/generic-ehci.yaml 
> > > b/Documentation/devicetree/bindings/usb/generic-ehci.yaml
> > > index 247ef00381ea..f76b25f7fc7a 100644
> > > --- a/Documentation/devicetree/bindings/usb/generic-ehci.yaml
> > > +++ b/Documentation/devicetree/bindings/usb/generic-ehci.yaml
> > > @@ -83,6 +83,7 @@ properties:
> > >Phandle of a companion.
> > >  
> > >phys:
> > > +maxItems: 1
> > >  description: PHY specifier for the USB PHY
> > >  
> > >phy-names:
> > > diff --git a/Documentation/devicetree/bindings/usb/generic-ohci.yaml 
> > > b/Documentation/devicetree/bindings/usb/generic-ohci.yaml
> > > index 2178bcc401bc..8e2bd61f2075 100644
> > > --- a/Documentation/devicetree/bindings/usb/generic-ohci.yaml
> > > +++ b/Documentation/devicetree/bindings/usb/generic-ohci.yaml
> > > @@ -71,6 +71,7 @@ properties:
> > >Overrides the detected port count
> > >  
> > >phys:
> > > +maxItems: 1
> > >  description: PHY specifier for the USB PHY
> > >  
> > >phy-names:
> > > diff --git a/Documentation/devicetree/bindings/usb/ingenic,musb.yaml 
> > > b/Documentation/devicetree/bindings/usb/ingenic,musb.yaml
> > > index 678396eeeb78..f506225a4d57 100644
> > > --- a/Documentation/devicetree/bindings/usb/ingenic,musb.yaml
> > > +++ b/Documentation/devicetree/bindings/usb/ingenic,musb.yaml
> > > @@ -40,7 +40,7 @@ properties:
> > >- const: mc
> > >  
> > >phys:
> > > -description: PHY specifier for the USB PHY
> > > +maxItems: 1
> > >  
> > >usb-role-switch:
> > >  type: boolean
> > 
> > Any reason you dropped the description for this entry, but not the other
> > ones above?
> 
> No, I should have dropped those too. I dropped cases of genericish 
> descriptions on common properties. There's nothing specific to this 
> binding here really.
> 
> > 
> > > diff --git a/Documentation/devicetree/bindings/usb/ti,j721e-usb.yaml 
> > > b/Documentation/devicetree/bindings/usb/ti,j721e-usb.yaml
> > > index 388245b91a55..adce36e48bc9 100644
> > > --- a/Documentation/devicetree/bindings/usb/ti,j721e-usb.yaml
> > > +++ b/Documentation/devicetree/bindings/usb/ti,j721e-usb.yaml
> > > @@ -15,13 +15,14 @@ properties:
> > >- const: ti,j721e-usb
> > >  
> > >reg:
> > > -description: module registers
> > > +maxItems: 1
> > >  
> > >power-domains:
> > >  description:
> > >PM domain provider node and an args specifier containing
> > >the USB device id value. See,
> > >Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
> > > +maxItems: 1
> > >  
> > >clocks:
> > >  description: Clock phandles to usb2_refclk and lpm_clk
> > 
> > Same here, why remove the description?
> 
> Really, the question is why keep 'description' on power-domains. Perhaps 
> there's a little value in the reference to sci-pm-domain.txt, so I left 
> it.

Ok, if you are fine with this, that's ok with me, just didn't look very
consistent :)

Re: [PATCH] dt-bindings: Add missing array size constraints

[Greg Kroah-Hartman]

On Mon, Jan 04, 2021 at 04:02:53PM -0700, Rob Herring wrote:
> DT properties which can have multiple entries need to specify what the
> entries are and define how many entries there can be. In the case of
> only a single entry, just 'maxItems: 1' is sufficient.
> 
> Add the missing entry constraints. These were found with a modified
> meta-schema. Unfortunately, there are a few cases where the size
> constraints are not defined such as common bindings, so the meta-schema
> can't be part of the normal checks.
> 
> Cc: Jens Axboe 
> Cc: Stephen Boyd 
> Cc: Thierry Reding 
> Cc: MyungJoo Ham 
> Cc: Chanwoo Choi 
> Cc: Linus Walleij 
> Cc: Bartosz Golaszewski 
> Cc: Jonathan Cameron 
> Cc: Dmitry Torokhov 
> Cc: Thomas Gleixner 
> Cc: Marc Zyngier 
> Cc: Mauro Carvalho Chehab 
> Cc: Chen-Yu Tsai 
> Cc: Ulf Hansson 
> Cc: "David S. Miller" 
> Cc: Jakub Kicinski 
> Cc: Sebastian Reichel 
> Cc: Ohad Ben-Cohen 
> Cc: Bjorn Andersson 
> Cc: Greg Kroah-Hartman 
> Signed-off-by: Rob Herring 



> diff --git a/Documentation/devicetree/bindings/usb/generic-ehci.yaml 
> b/Documentation/devicetree/bindings/usb/generic-ehci.yaml
> index 247ef00381ea..f76b25f7fc7a 100644
> --- a/Documentation/devicetree/bindings/usb/generic-ehci.yaml
> +++ b/Documentation/devicetree/bindings/usb/generic-ehci.yaml
> @@ -83,6 +83,7 @@ properties:
>Phandle of a companion.
>  
>phys:
> +maxItems: 1
>  description: PHY specifier for the USB PHY
>  
>phy-names:
> diff --git a/Documentation/devicetree/bindings/usb/generic-ohci.yaml 
> b/Documentation/devicetree/bindings/usb/generic-ohci.yaml
> index 2178bcc401bc..8e2bd61f2075 100644
> --- a/Documentation/devicetree/bindings/usb/generic-ohci.yaml
> +++ b/Documentation/devicetree/bindings/usb/generic-ohci.yaml
> @@ -71,6 +71,7 @@ properties:
>Overrides the detected port count
>  
>phys:
> +maxItems: 1
>  description: PHY specifier for the USB PHY
>  
>phy-names:
> diff --git a/Documentation/devicetree/bindings/usb/ingenic,musb.yaml 
> b/Documentation/devicetree/bindings/usb/ingenic,musb.yaml
> index 678396eeeb78..f506225a4d57 100644
> --- a/Documentation/devicetree/bindings/usb/ingenic,musb.yaml
> +++ b/Documentation/devicetree/bindings/usb/ingenic,musb.yaml
> @@ -40,7 +40,7 @@ properties:
>- const: mc
>  
>phys:
> -description: PHY specifier for the USB PHY
> +maxItems: 1
>  
>usb-role-switch:
>  type: boolean

Any reason you dropped the description for this entry, but not the other
ones above?

> diff --git a/Documentation/devicetree/bindings/usb/ti,j721e-usb.yaml 
> b/Documentation/devicetree/bindings/usb/ti,j721e-usb.yaml
> index 388245b91a55..adce36e48bc9 100644
> --- a/Documentation/devicetree/bindings/usb/ti,j721e-usb.yaml
> +++ b/Documentation/devicetree/bindings/usb/ti,j721e-usb.yaml
> @@ -15,13 +15,14 @@ properties:
>- const: ti,j721e-usb
>  
>reg:
> -description: module registers
> +maxItems: 1
>  
>power-domains:
>  description:
>PM domain provider node and an args specifier containing
>the USB device id value. See,
>Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
> +maxItems: 1
>  
>clocks:
>  description: Clock phandles to usb2_refclk and lpm_clk

Same here, why remove the description?

thanks,

greg k-h

Re: [PATCH v3 09/24] wfx: add hwio.c/hwio.h

[Greg Kroah-Hartman]

On Wed, Dec 23, 2020 at 09:01:33AM +0100, Jérôme Pouiller wrote:
> On Tuesday 22 December 2020 16:27:01 CET Greg Kroah-Hartman wrote:
> > On Tue, Dec 22, 2020 at 05:10:11PM +0200, Kalle Valo wrote:
> > > Jerome Pouiller  writes:
> > >
> > > > +/*
> > > > + * Internal helpers.
> > > > + *
> > > > + * About CONFIG_VMAP_STACK:
> > > > + * When CONFIG_VMAP_STACK is enabled, it is not possible to run DMA on 
> > > > stack
> > > > + * allocated data. Functions below that work with registers (aka 
> > > > functions
> > > > + * ending with "32") automatically reallocate buffers with kmalloc. 
> > > > However,
> > > > + * functions that work with arbitrary length buffers let's caller to 
> > > > handle
> > > > + * memory location. In doubt, enable CONFIG_DEBUG_SG to detect badly 
> > > > located
> > > > + * buffer.
> > > > + */
> > >
> > > This sounds very hacky to me, I have understood that you should never
> > > use stack with DMA.
> > 
> > You should never do that because some platforms do not support it, so no
> > driver should ever try to do that as they do not know what platform they
> > are running on.
> 
> Just to be curious, why these platforms don't support DMA in a stack
> allocated area?

Hardware is odd :(

> If the memory is contiguous (= not vmalloced), correctly
> aligned and in the first 4GB of physical memory, it should be sufficient,
> shouldn't?

Nope, sorry, this just does not work at all on many platforms.

thanks,

greg k-h

Re: [PATCH AUTOSEL 5.4 008/130] staging: wimax: depends on NET

[Greg Kroah-Hartman]

On Tue, Dec 22, 2020 at 09:16:11PM -0500, Sasha Levin wrote:
> From: Randy Dunlap 
> 
> [ Upstream commit 9364a2cf567187c0a075942c22d1f434c758de5d ]
> 
> Fix build errors when CONFIG_NET is not enabled. E.g. (trimmed):
> 
> ld: drivers/staging/wimax/op-msg.o: in function `wimax_msg_alloc':
> op-msg.c:(.text+0xa9): undefined reference to `__alloc_skb'
> ld: op-msg.c:(.text+0xcc): undefined reference to `genlmsg_put'
> ld: op-msg.c:(.text+0xfc): undefined reference to `nla_put'
> ld: op-msg.c:(.text+0x168): undefined reference to `kfree_skb'
> ld: drivers/staging/wimax/op-msg.o: in function `wimax_msg_data_len':
> op-msg.c:(.text+0x1ba): undefined reference to `nla_find'
> ld: drivers/staging/wimax/op-msg.o: in function `wimax_msg_send':
> op-msg.c:(.text+0x311): undefined reference to `init_net'
> ld: op-msg.c:(.text+0x326): undefined reference to `netlink_broadcast'
> ld: drivers/staging/wimax/stack.o: in function `__wimax_state_change':
> stack.c:(.text+0x433): undefined reference to `netif_carrier_off'
> ld: stack.c:(.text+0x46b): undefined reference to `netif_carrier_on'
> ld: stack.c:(.text+0x478): undefined reference to `netif_tx_wake_queue'
> ld: drivers/staging/wimax/stack.o: in function `wimax_subsys_exit':
> stack.c:(.exit.text+0xe): undefined reference to `genl_unregister_family'
> ld: drivers/staging/wimax/stack.o: in function `wimax_subsys_init':
> stack.c:(.init.text+0x1a): undefined reference to `genl_register_family'
> 
> Cc: Greg Kroah-Hartman 
> Cc: Jakub Kicinski 
> Cc: Arnd Bergmann 
> Cc: netdev@vger.kernel.org
> Acked-by: Arnd Bergmann 
> Signed-off-by: Randy Dunlap 
> Link: https://lore.kernel.org/r/20201102072456.20303-1-rdun...@infradead.org
> Signed-off-by: Greg Kroah-Hartman 
> Signed-off-by: Sasha Levin 
> ---
>  net/wimax/Kconfig | 1 +
>  1 file changed, 1 insertion(+)

This isn't needed in any backported kernel as it only is relevant when
the code moved to drivers/staging/

thanks,

greg k-h

Re: [PATCH v3 09/24] wfx: add hwio.c/hwio.h

[Greg Kroah-Hartman]

On Tue, Dec 22, 2020 at 05:10:11PM +0200, Kalle Valo wrote:
> Jerome Pouiller  writes:
> 
> > +/*
> > + * Internal helpers.
> > + *
> > + * About CONFIG_VMAP_STACK:
> > + * When CONFIG_VMAP_STACK is enabled, it is not possible to run DMA on 
> > stack
> > + * allocated data. Functions below that work with registers (aka functions
> > + * ending with "32") automatically reallocate buffers with kmalloc. 
> > However,
> > + * functions that work with arbitrary length buffers let's caller to handle
> > + * memory location. In doubt, enable CONFIG_DEBUG_SG to detect badly 
> > located
> > + * buffer.
> > + */
> 
> This sounds very hacky to me, I have understood that you should never
> use stack with DMA.

You should never do that because some platforms do not support it, so no
driver should ever try to do that as they do not know what platform they
are running on.

thanks,

greg k-h

Re: [PATCH v18 0/3] userspace MHI client interface driver

[Greg Kroah-Hartman]

On Wed, Dec 16, 2020 at 10:17:30AM +0100, Loic Poulain wrote:
> Hi Folks,
> 
> On Fri, 11 Dec 2020 at 20:45, Hemant Kumar  wrote:
> >
> > This patch series adds support for UCI driver. UCI driver enables userspace
> > clients to communicate to external MHI devices like modem. UCI driver probe
> > creates standard character device file nodes for userspace clients to
> > perform open, read, write, poll and release file operations. These file
> > operations call MHI core layer APIs to perform data transfer using MHI bus
> > to communicate with MHI device.
> >
> > This interface allows exposing modem control channel(s) such as QMI, MBIM,
> > or AT commands to userspace which can be used to configure the modem using
> > tools such as libqmi, ModemManager, minicom (for AT), etc over MHI. This is
> > required as there are no kernel APIs to access modem control path for device
> > configuration. Data path transporting the network payload (IP), however, is
> > routed to the Linux network via the mhi-net driver. Currently driver 
> > supports
> > QMI channel. libqmi is userspace MHI client which communicates to a QMI
> > service using QMI channel. Please refer to
> > https://www.freedesktop.org/wiki/Software/libqmi/ for additional information
> > on libqmi.
> >
> > Patch is tested using arm64 and x86 based platform.
> 
> Are there any blockers or unadressed comments remaining on this
> series? As far as I understand, the original blocker was the net/WiFi
> mention in the commit message, that caused a legitimate concern from
> network maintainer. It has been clarified now that this driver is not
> for exposing any channel that could be otherwise handled properly by
> an existing Linux subsystem/interface. It will be especially used as a
> pipe for modem QMI channel (or AT commands) in the same way as the USB
> CDC-WDM driver is doing (keeping userspace compatibility). Other MHI
> channels, such as network data, QRTR, etc are not exposed and
> correctly bound to the corresponding Linux subsystems.
> 
> The correlated worry was that it could be a userspace channel facility
> for 'everything qualcomm', but we could say the same for other
> existing busses with userspace shunt (/dev/bus/usb, /dev/i2c,
> /dev/spidev, PCI UIO, UART...). Moreover, it is mitigated by the fact
> that not all MHI channels are exposed by default, but only the allowed
> ones (QMI in the initial version). For sure, special care must be
> given to any further channel addition.

It's the middle of the merge window, we can't do anything with new
patches at all until 5.11-rc1 is out, so please be patient.

thanks,

greg k-h

[PATCH 5.9 031/105] net: broadcom CNIC: requires MMU

[Greg Kroah-Hartman]

From: Randy Dunlap 

[ Upstream commit 14483cbf040fcb38113497161088a1ce8ce5d713 ]

The CNIC kconfig symbol selects UIO and UIO depends on MMU.
Since 'select' does not follow dependency chains, add the same MMU
dependency to CNIC.

Quietens this kconfig warning:

WARNING: unmet direct dependencies detected for UIO
  Depends on [n]: MMU [=n]
  Selected by [m]:
  - CNIC [=m] && NETDEVICES [=y] && ETHERNET [=y] && NET_VENDOR_BROADCOM [=y] 
&& PCI [=y] && (IPV6 [=m] || IPV6 [=m]=n)

Fixes: adfc5217e9db ("broadcom: Move the Broadcom drivers")
Signed-off-by: Randy Dunlap 
Cc: Jeff Kirsher 
Cc: Rasesh Mody 
Cc: gr-linux-nic-...@marvell.com
Cc: "David S. Miller" 
Cc: Jakub Kicinski 
Cc: netdev@vger.kernel.org
Link: https://lore.kernel.org/r/20201129070843.3859-1-rdun...@infradead.org
Signed-off-by: Jakub Kicinski 
Signed-off-by: Sasha Levin 
---
 drivers/net/ethernet/broadcom/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/broadcom/Kconfig 
b/drivers/net/ethernet/broadcom/Kconfig
index 7fb42f388d591..7b79528d6eed2 100644
--- a/drivers/net/ethernet/broadcom/Kconfig
+++ b/drivers/net/ethernet/broadcom/Kconfig
@@ -88,6 +88,7 @@ config BNX2
 config CNIC
tristate "QLogic CNIC support"
depends on PCI && (IPV6 || IPV6=n)
+   depends on MMU
select BNX2
select UIO
help
-- 
2.27.0

[PATCH 5.9 032/105] vdpa: mlx5: fix vdpa/vhost dependencies

[Greg Kroah-Hartman]

From: Randy Dunlap 

[ Upstream commit 98701a2a861fa87a5055cf2809758e8725e8b146 ]

drivers/vdpa/mlx5/ uses vhost_iotlb*() interfaces, so select
VHOST_IOTLB to make them be built.

However, if VHOST_IOTLB is the only VHOST symbol that is
set/enabled, the object file still won't be built because
drivers/Makefile won't descend into drivers/vhost/ to build it,
so make drivers/Makefile build the needed binary whenever
VHOST_IOTLB is set, like it does for VHOST_RING.

Fixes these build errors:
ERROR: modpost: "vhost_iotlb_itree_next" [drivers/vdpa/mlx5/mlx5_vdpa.ko] 
undefined!
ERROR: modpost: "vhost_iotlb_itree_first" [drivers/vdpa/mlx5/mlx5_vdpa.ko] 
undefined!

Fixes: 29064bfdabd5 ("vdpa/mlx5: Add support library for mlx5 VDPA 
implementation")
Fixes: aff90770e54c ("vdpa/mlx5: Fix dependency on MLX5_CORE")
Reported-by: kernel test robot 
Signed-off-by: Randy Dunlap 
Cc: Eli Cohen 
Cc: Parav Pandit 
Cc: "Michael S. Tsirkin" 
Cc: Jason Wang 
Cc: virtualizat...@lists.linux-foundation.org
Cc: Saeed Mahameed 
Cc: Leon Romanovsky 
Cc: netdev@vger.kernel.org
Link: https://lore.kernel.org/r/20201128213905.27409-1-rdun...@infradead.org
Signed-off-by: Michael S. Tsirkin 
Acked-by: Jason Wang 
Signed-off-by: Sasha Levin 
---
 drivers/Makefile | 1 +
 drivers/vdpa/Kconfig | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/Makefile b/drivers/Makefile
index c0cd1b9075e3d..5762280377186 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -145,6 +145,7 @@ obj-$(CONFIG_OF)+= of/
 obj-$(CONFIG_SSB)  += ssb/
 obj-$(CONFIG_BCMA) += bcma/
 obj-$(CONFIG_VHOST_RING)   += vhost/
+obj-$(CONFIG_VHOST_IOTLB)  += vhost/
 obj-$(CONFIG_VHOST)+= vhost/
 obj-$(CONFIG_VLYNQ)+= vlynq/
 obj-$(CONFIG_GREYBUS)  += greybus/
diff --git a/drivers/vdpa/Kconfig b/drivers/vdpa/Kconfig
index 358f6048dd3ce..6caf539091e55 100644
--- a/drivers/vdpa/Kconfig
+++ b/drivers/vdpa/Kconfig
@@ -32,6 +32,7 @@ config IFCVF
 
 config MLX5_VDPA
bool
+   select VHOST_IOTLB
help
  Support library for Mellanox VDPA drivers. Provides code that is
  common for all types of VDPA drivers. The following drivers are 
planned:
-- 
2.27.0

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Greg Kroah-Hartman]

On Tue, Dec 08, 2020 at 04:02:50PM +0100, Marcin Wojtas wrote:
> Hi Sasha,
> 
> wt., 8 gru 2020 o 14:35 Sasha Levin  napisał(a):
> >
> > On Tue, Dec 08, 2020 at 01:03:38PM +0100, Marcin Wojtas wrote:
> > >Hi Greg,
> > >
> > >Apologies for delayed response:.
> > >
> > >
> > >pon., 2 lis 2020 o 19:02 Greg Kroah-Hartman
> > > napisał(a):
> > >>
> > >> On Mon, Nov 02, 2020 at 06:38:54PM +0100, Marcin Wojtas wrote:
> > >> > Hi Greg and Sasha,
> > >> >
> > >> > pt., 9 paź 2020 o 05:43 Marcin Wojtas  napisał(a):
> > >> > >
> > >> > > Hi,
> > >> > >
> > >> > > sob., 20 cze 2020 o 11:21 Russell King  
> > >> > > napisał(a):
> > >> > > >
> > >> > > > Add a helper to convert the struct phylink_config pointer passed in
> > >> > > > from phylink to the drivers internal struct mvpp2_port.
> > >> > > >
> > >> > > > Signed-off-by: Russell King 
> > >> > > > ---
> > >> > > >  .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 29 
> > >> > > > +--
> > >> > > >  1 file changed, 14 insertions(+), 15 deletions(-)
> > >> > > >
> > >> > > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c 
> > >> > > > b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > >> > > > index 7653277d03b7..313f5a60a605 100644
> > >> > > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > >> > > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > >> > > > @@ -4767,12 +4767,16 @@ static void 
> > >> > > > mvpp2_port_copy_mac_addr(struct net_device *dev, struct mvpp2 
> > >> > > > *priv,
> > >> > > > eth_hw_addr_random(dev);
> > >> > > >  }
> > >> > > >
> > >> > > > +static struct mvpp2_port *mvpp2_phylink_to_port(struct 
> > >> > > > phylink_config *config)
> > >> > > > +{
> > >> > > > +   return container_of(config, struct mvpp2_port, 
> > >> > > > phylink_config);
> > >> > > > +}
> > >> > > > +
> > >> > > >  static void mvpp2_phylink_validate(struct phylink_config *config,
> > >> > > >unsigned long *supported,
> > >> > > >struct phylink_link_state 
> > >> > > > *state)
> > >> > > >  {
> > >> > > > -   struct mvpp2_port *port = container_of(config, struct 
> > >> > > > mvpp2_port,
> > >> > > > -  phylink_config);
> > >> > > > +   struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > >> > > > __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> > >> > > >
> > >> > > > /* Invalid combinations */
> > >> > > > @@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct 
> > >> > > > mvpp2_port *port,
> > >> > > >  static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config 
> > >> > > > *config,
> > >> > > > struct 
> > >> > > > phylink_link_state *state)
> > >> > > >  {
> > >> > > > -   struct mvpp2_port *port = container_of(config, struct 
> > >> > > > mvpp2_port,
> > >> > > > -  phylink_config);
> > >> > > > +   struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > >> > > >
> > >> > > > if (port->priv->hw_version == MVPP22 && port->gop_id == 0) 
> > >> > > > {
> > >> > > > u32 mode = readl(port->base + 
> > >> > > > MVPP22_XLG_CTRL3_REG);
> > >> > > > @@ -4931,8 +4934,7 @@ static void 
> > >> > > > mvpp2_phylink_mac_pcs_get_state(struct phylink_config *config,
> > >> > > >
> > >> > > >  st

[PATCH 4.19 02/57] netfilter: clear skb->next in NF_HOOK_LIST()

[Greg Kroah-Hartman]

From: Cong Wang 

NF_HOOK_LIST() uses list_del() to remove skb from the linked list,
however, it is not sufficient as skb->next still points to other
skb. We should just call skb_list_del_init() to clear skb->next,
like the rest places which using skb list.

This has been fixed in upstream by commit ca58fbe06c54
("netfilter: add and use nf_hook_slow_list()").

Fixes: 9f17dbf04ddf ("netfilter: fix use-after-free in NF_HOOK_LIST")
Reported-by: li...@knownsec.com
Tested-by: li...@knownsec.com
Cc: Florian Westphal 
Cc: Edward Cree 
Cc: sta...@vger.kernel.org # between 4.19 and 5.4
Signed-off-by: Cong Wang 
Signed-off-by: Greg Kroah-Hartman 

---
 include/linux/netfilter.h |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -300,7 +300,7 @@ NF_HOOK_LIST(uint8_t pf, unsigned int ho
 
INIT_LIST_HEAD(&sublist);
list_for_each_entry_safe(skb, next, head, list) {
-   list_del(&skb->list);
+   skb_list_del_init(skb);
if (nf_hook(pf, hook, net, sk, skb, in, out, okfn) == 1)
list_add_tail(&skb->list, &sublist);
}

[PATCH 5.4 04/98] netfilter: clear skb->next in NF_HOOK_LIST()

[Greg Kroah-Hartman]

From: Cong Wang 

NF_HOOK_LIST() uses list_del() to remove skb from the linked list,
however, it is not sufficient as skb->next still points to other
skb. We should just call skb_list_del_init() to clear skb->next,
like the rest places which using skb list.

This has been fixed in upstream by commit ca58fbe06c54
("netfilter: add and use nf_hook_slow_list()").

Fixes: 9f17dbf04ddf ("netfilter: fix use-after-free in NF_HOOK_LIST")
Reported-by: li...@knownsec.com
Tested-by: li...@knownsec.com
Cc: Florian Westphal 
Cc: Edward Cree 
Cc: sta...@vger.kernel.org # between 4.19 and 5.4
Signed-off-by: Cong Wang 
Signed-off-by: Greg Kroah-Hartman 

---
 include/linux/netfilter.h |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -316,7 +316,7 @@ NF_HOOK_LIST(uint8_t pf, unsigned int ho
 
INIT_LIST_HEAD(&sublist);
list_for_each_entry_safe(skb, next, head, list) {
-   list_del(&skb->list);
+   skb_list_del_init(skb);
if (nf_hook(pf, hook, net, sk, skb, in, out, okfn) == 1)
list_add_tail(&skb->list, &sublist);
}

Re: [Patch stable] netfilter: clear skb->next in NF_HOOK_LIST()

[Greg Kroah-Hartman]

On Sat, Nov 28, 2020 at 05:09:18PM +0800, 刘志旭 wrote:
> I still didn't see this patch in stable queue yet. Since we've a working POC 
> to panic the 
> system (see https://bugzilla.kernel.org/show_bug.cgi?id=209823), I think it's 
> necessary
> to merge this patch ASAP, thanks.

Odd, I don't think Sasha pushed out his patch queue.  I've applied this
now, thanks.

greg k-h

Re: [PATCH v12 1/5] bus: mhi: core: Add helper API to return number of free TREs

[Greg Kroah-Hartman]

On Sat, Nov 28, 2020 at 11:59:46AM +0530, Manivannan Sadhasivam wrote:
> On Wed, Nov 18, 2020 at 10:32:45AM +0100, Loic Poulain wrote:
> > On Mon, 16 Nov 2020 at 23:46, Hemant Kumar  wrote:
> > >
> > > Introduce mhi_get_free_desc_count() API to return number
> > > of TREs available to queue buffer. MHI clients can use this
> > > API to know before hand if ring is full without calling queue
> > > API.
> > >
> > > Signed-off-by: Hemant Kumar 
> > > Reviewed-by: Jeffrey Hugo 
> > > Reviewed-by: Manivannan Sadhasivam 
> > 
> > In case this series get new comments to address, I would suggest
> > merging that patch in mhi-next separately so that other drivers can
> > start benefiting this function (I would like to use it in mhi-net).
> > 
> 
> Greg doesn't like that. He asked me to pick APIs only when there an in-tree
> consumer available.

If someone wants to use it, then yes, by all means merge it.  I can't
just take new apis without any user.

thanks,

greg k-h

Re: [PATCHv3] bonding: wait for sysfs kobject destruction before freeing struct slave

[Greg Kroah-Hartman]

On Fri, Nov 20, 2020 at 02:28:27PM +, Jamie Iles wrote:
> syzkaller found that with CONFIG_DEBUG_KOBJECT_RELEASE=y, releasing a
> struct slave device could result in the following splat:
> 
>   kobject: 'bonding_slave' (cecdd4fe): kobject_release, parent 
> 74ceb2b2 (delayed 1000)
>   bond0 (unregistering): (slave bond_slave_1): Releasing backup interface
>   [ cut here ]
>   ODEBUG: free active (active state 0) object type: timer_list hint: 
> workqueue_select_cpu_near kernel/workqueue.c:1549 [inline]
>   ODEBUG: free active (active state 0) object type: timer_list hint: 
> delayed_work_timer_fn+0x0/0x98 kernel/workqueue.c:1600
>   WARNING: CPU: 1 PID: 842 at lib/debugobjects.c:485 
> debug_print_object+0x180/0x240 lib/debugobjects.c:485
>   Kernel panic - not syncing: panic_on_warn set ...
>   CPU: 1 PID: 842 Comm: kworker/u4:4 Tainted: G S5.9.0-rc8+ 
> #96
>   Hardware name: linux,dummy-virt (DT)
>   Workqueue: netns cleanup_net
>   Call trace:
>dump_backtrace+0x0/0x4d8 include/linux/bitmap.h:239
>show_stack+0x34/0x48 arch/arm64/kernel/traps.c:142
>__dump_stack lib/dump_stack.c:77 [inline]
>dump_stack+0x174/0x1f8 lib/dump_stack.c:118
>panic+0x360/0x7a0 kernel/panic.c:231
>__warn+0x244/0x2ec kernel/panic.c:600
>report_bug+0x240/0x398 lib/bug.c:198
>bug_handler+0x50/0xc0 arch/arm64/kernel/traps.c:974
>call_break_hook+0x160/0x1d8 arch/arm64/kernel/debug-monitors.c:322
>brk_handler+0x30/0xc0 arch/arm64/kernel/debug-monitors.c:329
>do_debug_exception+0x184/0x340 arch/arm64/mm/fault.c:864
>el1_dbg+0x48/0xb0 arch/arm64/kernel/entry-common.c:65
>el1_sync_handler+0x170/0x1c8 arch/arm64/kernel/entry-common.c:93
>el1_sync+0x80/0x100 arch/arm64/kernel/entry.S:594
>debug_print_object+0x180/0x240 lib/debugobjects.c:485
>__debug_check_no_obj_freed lib/debugobjects.c:967 [inline]
>debug_check_no_obj_freed+0x200/0x430 lib/debugobjects.c:998
>slab_free_hook mm/slub.c:1536 [inline]
>slab_free_freelist_hook+0x190/0x210 mm/slub.c:1577
>slab_free mm/slub.c:3138 [inline]
>kfree+0x13c/0x460 mm/slub.c:4119
>bond_free_slave+0x8c/0xf8 drivers/net/bonding/bond_main.c:1492
>__bond_release_one+0xe0c/0xec8 drivers/net/bonding/bond_main.c:2190
>bond_slave_netdev_event drivers/net/bonding/bond_main.c:3309 [inline]
>bond_netdev_event+0x8f0/0xa70 drivers/net/bonding/bond_main.c:3420
>notifier_call_chain+0xf0/0x200 kernel/notifier.c:83
>__raw_notifier_call_chain kernel/notifier.c:361 [inline]
>raw_notifier_call_chain+0x44/0x58 kernel/notifier.c:368
>call_netdevice_notifiers_info+0xbc/0x150 net/core/dev.c:2033
>call_netdevice_notifiers_extack net/core/dev.c:2045 [inline]
>call_netdevice_notifiers net/core/dev.c:2059 [inline]
>rollback_registered_many+0x6a4/0xec0 net/core/dev.c:9347
>unregister_netdevice_many.part.0+0x2c/0x1c0 net/core/dev.c:10509
>unregister_netdevice_many net/core/dev.c:10508 [inline]
>default_device_exit_batch+0x294/0x338 net/core/dev.c:10992
>ops_exit_list.isra.0+0xec/0x150 net/core/net_namespace.c:189
>cleanup_net+0x44c/0x888 net/core/net_namespace.c:603
>process_one_work+0x96c/0x18c0 kernel/workqueue.c:2269
>worker_thread+0x3f0/0xc30 kernel/workqueue.c:2415
>kthread+0x390/0x498 kernel/kthread.c:292
>ret_from_fork+0x10/0x18 arch/arm64/kernel/entry.S:925
> 
> This is a potential use-after-free if the sysfs nodes are being accessed
> whilst removing the struct slave, so wait for the object destruction to
> complete before freeing the struct slave itself.
> 
> Fixes: 07699f9a7c8d ("bonding: add sysfs /slave dir for bond slave devices.")
> Fixes: a068aab42258 ("bonding: Fix reference count leak in 
> bond_sysfs_slave_add.")
> Cc: Greg Kroah-Hartman 
> Cc: Qiushi Wu 
> Cc: Jay Vosburgh 
> Cc: Veaceslav Falico 
> Cc: Andy Gospodarek 
> Signed-off-by: Jamie Iles 
> ---
> v3:
>  - have a single object lifecycle in the struct slave and remove the
>explicit deallocation and defer that to the kobject

Nice, it looks like it should have always been done this way!

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH 5.9 000/255] 5.9.9-rc1 review

[Greg Kroah-Hartman]

On Wed, Nov 18, 2020 at 11:03:55AM +0530, Naresh Kamboju wrote:
> On Tue, 17 Nov 2020 at 19:02, Greg Kroah-Hartman
>  wrote:
> >
> > This is the start of the stable review cycle for the 5.9.9 release.
> > There are 255 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 Thu, 19 Nov 2020 12:20:51 +.
> > Anything received after that time might be too late.
> >
> > The whole patch series can be found in one patch at:
> > 
> > https://www.kernel.org/pub/linux/kernel/v5.x/stable-review/patch-5.9.9-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-5.9.y
> > and the diffstat can be found below.
> >
> > thanks,
> >
> > greg k-h
> 
> Results from Linaro’s test farm.
> No regressions on arm64, arm, x86_64, and i386.
> 
> Tested-by: Linux Kernel Functional Testing 

Thanks for testing all of these and letting me know.

greg k-h

Re: [PATCHv2] bonding: wait for sysfs kobject destruction before freeing struct slave

[Greg Kroah-Hartman]

On Tue, Nov 17, 2020 at 12:34:01PM -0800, Jakub Kicinski wrote:
> On Fri, 13 Nov 2020 17:12:44 + Jamie Iles wrote:
> > syzkaller found that with CONFIG_DEBUG_KOBJECT_RELEASE=y, releasing a
> > struct slave device could result in the following splat:
> 
> > This is a potential use-after-free if the sysfs nodes are being accessed
> > whilst removing the struct slave, so wait for the object destruction to
> > complete before freeing the struct slave itself.
> > 
> > Fixes: 07699f9a7c8d ("bonding: add sysfs /slave dir for bond slave 
> > devices.")
> > Fixes: a068aab42258 ("bonding: Fix reference count leak in 
> > bond_sysfs_slave_add.")
> 
> This code looks surprising, although admittedly my kobj understanding
> is cursory at best. So CCing Greg to keep me honest.
> 
> kobj itself is a refcounting mechanism. Adding another refcount and
> then releasing the reference from .release method of kobject looks like
> a pointless duplication.

Yes, that's wrong.

> Just free the object from the .release method. Why not?

Correct, that is what should be happening.  Otherwise it seems that
something else has a pointer to this object that forgot to increment it?

> 
> > diff --git a/drivers/net/bonding/bond_main.c 
> > b/drivers/net/bonding/bond_main.c
> > index 84ecbc6fa0ff..66e56642e6c2 100644
> > --- a/drivers/net/bonding/bond_main.c
> > +++ b/drivers/net/bonding/bond_main.c
> > @@ -1478,11 +1478,14 @@ static struct slave *bond_alloc_slave(struct 
> > bonding *bond)
> > }
> > INIT_DELAYED_WORK(&slave->notify_work, bond_netdev_notify_work);
> >  
> > +   kref_init(&slave->ref);
> > +
> > return slave;
> >  }
> >  
> > -static void bond_free_slave(struct slave *slave)
> > +static void __bond_free_slave(struct kref *ref)
> >  {
> > +   struct slave *slave = container_of(ref, struct slave, ref);
> > struct bonding *bond = bond_get_bond_by_slave(slave);
> >  
> > cancel_delayed_work_sync(&slave->notify_work);
> > @@ -1492,6 +1495,16 @@ static void bond_free_slave(struct slave *slave)
> > kfree(slave);
> >  }
> >  
> > +void bond_slave_put_ref(struct slave *slave)
> > +{
> > +   kref_put(&slave->ref, __bond_free_slave);
> > +}
> > +
> > +void bond_slave_get_ref(struct slave *slave)
> > +{
> > +   kref_get(&slave->ref);
> > +}
> > +
> >  static void bond_fill_ifbond(struct bonding *bond, struct ifbond *info)
> >  {
> > info->bond_mode = BOND_MODE(bond);
> > @@ -2007,7 +2020,7 @@ int bond_enslave(struct net_device *bond_dev, struct 
> > net_device *slave_dev,
> > dev_set_mtu(slave_dev, new_slave->original_mtu);
> >  
> >  err_free:
> > -   bond_free_slave(new_slave);
> > +   bond_slave_put_ref(new_slave);
> >  
> >  err_undo_flags:
> > /* Enslave of first slave has failed and we need to fix master's mac */
> > @@ -2187,7 +2200,7 @@ static int __bond_release_one(struct net_device 
> > *bond_dev,
> > if (!netif_is_bond_master(slave_dev))
> > slave_dev->priv_flags &= ~IFF_BONDING;
> >  
> > -   bond_free_slave(slave);
> > +   bond_slave_put_ref(slave);
> >  
> > return 0;
> >  }
> > diff --git a/drivers/net/bonding/bond_sysfs_slave.c 
> > b/drivers/net/bonding/bond_sysfs_slave.c
> > index 9b8346638f69..5f8aac715ee8 100644
> > --- a/drivers/net/bonding/bond_sysfs_slave.c
> > +++ b/drivers/net/bonding/bond_sysfs_slave.c
> > @@ -136,7 +136,15 @@ static const struct sysfs_ops slave_sysfs_ops = {
> > .show = slave_show,
> >  };
> >  
> > +static void slave_release(struct kobject *kobj)
> > +{
> > +   struct slave *slave = to_slave(kobj);
> > +
> > +   bond_slave_put_ref(slave);
> > +}
> > +
> >  static struct kobj_type slave_ktype = {
> > +   .release = slave_release,
> >  #ifdef CONFIG_SYSFS
> > .sysfs_ops = &slave_sysfs_ops,
> >  #endif
> > @@ -147,22 +155,26 @@ int bond_sysfs_slave_add(struct slave *slave)
> > const struct slave_attribute **a;
> > int err;
> >  
> > +   bond_slave_get_ref(slave);
> > +
> > err = kobject_init_and_add(&slave->kobj, &slave_ktype,
> >&(slave->dev->dev.kobj), "bonding_slave");
> > -   if (err) {
> > -   kobject_put(&slave->kobj);
> > -   return err;
> > -   }
> > +   if (err)
> > +   goto out_put_slave;
> >  
> > for (a = slave_attrs; *a; ++a) {
> > err = sysfs_create_file(&slave->kobj, &((*a)->attr));
> > -   if (err) {
> > -   kobject_put(&slave->kobj);
> > -   return err;
> > -   }
> > +   if (err)
> > +   goto out_put_slave;
> > }
> >  
> > return 0;
> > +
> > +out_put_slave:
> > +   kobject_put(&slave->kobj);
> > +   bond_slave_put_ref(slave);
> > +
> > +   return err;
> >  }
> >  
> >  void bond_sysfs_slave_del(struct slave *slave)
> > diff --git a/include/net/bonding.h b/include/net/bonding.h
> > index 7d132cc1e584..e286ff4e0882 100644
> > --- a/include/net/bonding.h
> > +++ b/include/net/bonding.h
> > @@ -25,6 +25,7 @@
> >  #include 
> >  #include 
> >  #include 
>

Re: [PATCH v2 00/10] thunderbolt: Add DMA traffic test driver

[Greg Kroah-Hartman]

On Tue, Nov 10, 2020 at 12:19:47PM +0300, Mika Westerberg wrote:
> Hi all,
> 
> This series adds a new Thunderbolt service driver that can be used on
> manufacturing floor to test that each Thunderbolt/USB4 port is functional.
> It can be done either using a special loopback dongle that has RX and TX
> lanes crossed, or by connecting a cable back to the host (for those who
> don't have these dongles).
> 
> This takes advantage of the existing XDomain protocol and creates XDomain
> devices for the loops back to the host where the DMA traffic test driver
> can bind to.
> 
> The DMA traffic test driver creates a tunnel through the fabric and then
> sends and receives data frames over the tunnel checking for different
> errors.
> 
> The previous version can be found here:
> 
>   
> https://lore.kernel.org/linux-usb/20201104140030.6853-1-mika.westerb...@linux.intel.com/
> 
> Changes from the previous version:
> 
>   * Fix resource leak in tb_xdp_handle_request() (patch 2/10)
>   * Use debugfs_remove_recursive() in tb_service_debugfs_remove() (patch 6/10)
>   * Add tags from Yehezkel

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH v2 bpf-next 4/5] bpf: load and verify kernel module BTFs

[Greg Kroah-Hartman]

On Fri, Nov 06, 2020 at 03:02:27PM -0800, Andrii Nakryiko wrote:
> Add kernel module listener that will load/validate and unload module BTF.
> Module BTFs gets ID generated for them, which makes it possible to iterate
> them with existing BTF iteration API. They are given their respective module's
> names, which will get reported through GET_OBJ_INFO API. They are also marked
> as in-kernel BTFs for tooling to distinguish them from user-provided BTFs.
> 
> Also, similarly to vmlinux BTF, kernel module BTFs are exposed through
> sysfs as /sys/kernel/btf/. This is convenient for user-space
> tools to inspect module BTF contents and dump their types with existing tools:
> 
> [vmuser@archvm bpf]$ ls -la /sys/kernel/btf
> total 0
> drwxr-xr-x  2 root root   0 Nov  4 19:46 .
> drwxr-xr-x 13 root root   0 Nov  4 19:46 ..
> 
> ...
> 
> -r--r--r--  1 root root 888 Nov  4 19:46 irqbypass
> -r--r--r--  1 root root  100225 Nov  4 19:46 kvm
> -r--r--r--  1 root root   35401 Nov  4 19:46 kvm_intel
> -r--r--r--  1 root root 120 Nov  4 19:46 pcspkr
> -r--r--r--  1 root root 399 Nov  4 19:46 serio_raw
> -r--r--r--  1 root root 4094095 Nov  4 19:46 vmlinux
> 
> Signed-off-by: Andrii Nakryiko 
> ---
>  Documentation/ABI/testing/sysfs-kernel-btf |   8 +
>  include/linux/bpf.h|   2 +
>  include/linux/module.h |   4 +
>  kernel/bpf/btf.c   | 194 +
>  kernel/bpf/sysfs_btf.c |   2 +-
>  kernel/module.c        |  32 
>  6 files changed, 241 insertions(+), 1 deletion(-)

Reviewed-by: Greg Kroah-Hartman 

Re: [PATCH bpf-next 4/5] bpf: load and verify kernel module BTFs

[Greg Kroah-Hartman]

On Thu, Nov 05, 2020 at 09:51:09PM -0800, Andrii Nakryiko wrote:
> Add kernel module listener that will load/validate and unload module BTF.
> Module BTFs gets ID generated for them, which makes it possible to iterate
> them with existing BTF iteration API. They are given their respective module's
> names, which will get reported through GET_OBJ_INFO API. They are also marked
> as in-kernel BTFs for tooling to distinguish them from user-provided BTFs.
> 
> Also, similarly to vmlinux BTF, kernel module BTFs are exposed through
> sysfs as /sys/kernel/btf/. This is convenient for user-space
> tools to inspect module BTF contents and dump their types with existing tools:
> 
> [vmuser@archvm bpf]$ ls -la /sys/kernel/btf
> total 0
> drwxr-xr-x  2 root root   0 Nov  4 19:46 .
> drwxr-xr-x 13 root root   0 Nov  4 19:46 ..
> 
> ...
> 
> -r--r--r--  1 root root 888 Nov  4 19:46 irqbypass
> -r--r--r--  1 root root  100225 Nov  4 19:46 kvm
> -r--r--r--  1 root root   35401 Nov  4 19:46 kvm_intel
> -r--r--r--  1 root root 120 Nov  4 19:46 pcspkr
> -r--r--r--  1 root root 399 Nov  4 19:46 serio_raw
> -r--r--r--  1 root root 4094095 Nov  4 19:46 vmlinux
> 
> Signed-off-by: Andrii Nakryiko 
> ---
>  Documentation/ABI/testing/sysfs-kernel-btf |   8 +
>  include/linux/bpf.h|   2 +
>  include/linux/module.h |   4 +
>  kernel/bpf/btf.c   | 193 +
>  kernel/bpf/sysfs_btf.c |   2 +-
>  kernel/module.c|  32 
>  6 files changed, 240 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-kernel-btf 
> b/Documentation/ABI/testing/sysfs-kernel-btf
> index 2c9744b2cd59..fe96efdc9b6c 100644
> --- a/Documentation/ABI/testing/sysfs-kernel-btf
> +++ b/Documentation/ABI/testing/sysfs-kernel-btf
> @@ -15,3 +15,11 @@ Description:
>   information with description of all internal kernel types. See
>   Documentation/bpf/btf.rst for detailed description of format
>   itself.
> +
> +What:/sys/kernel/btf/
> +Date:Nov 2020
> +KernelVersion:   5.11
> +Contact: b...@vger.kernel.org
> +Description:
> + Read-only binary attribute exposing kernel module's BTF type
> + information as an add-on to the kernel's BTF 
> (/sys/kernel/btf/vmlinux).
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 2fffd30e13ac..3cb89cd7177b 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -36,9 +36,11 @@ struct seq_operations;
>  struct bpf_iter_aux_info;
>  struct bpf_local_storage;
>  struct bpf_local_storage_map;
> +struct kobject;
>  
>  extern struct idr btf_idr;
>  extern spinlock_t btf_idr_lock;
> +extern struct kobject *btf_kobj;
>  
>  typedef int (*bpf_iter_init_seq_priv_t)(void *private_data,
>   struct bpf_iter_aux_info *aux);
> diff --git a/include/linux/module.h b/include/linux/module.h
> index a29187f7c360..20fce258ffba 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -475,6 +475,10 @@ struct module {
>   unsigned int num_bpf_raw_events;
>   struct bpf_raw_event_map *bpf_raw_events;
>  #endif
> +#ifdef CONFIG_DEBUG_INFO_BTF_MODULES
> + unsigned int btf_data_size;
> + void *btf_data;
> +#endif
>  #ifdef CONFIG_JUMP_LABEL
>   struct jump_entry *jump_entries;
>   unsigned int num_jump_entries;
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 09f1483934d2..fe639ffae361 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -23,6 +23,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  
>  /* BTF (BPF Type Format) is the meta data format which describes
> @@ -4488,6 +4490,75 @@ struct btf *btf_parse_vmlinux(void)
>   return ERR_PTR(err);
>  }
>  
> +static struct btf *btf_parse_module(const char *module_name, const void 
> *data, unsigned int data_size)
> +{
> + struct btf_verifier_env *env = NULL;
> + struct bpf_verifier_log *log;
> + struct btf *btf = NULL, *base_btf;
> + int err;
> +
> + base_btf = bpf_get_btf_vmlinux();
> + if (IS_ERR(base_btf))
> + return base_btf;
> + if (!base_btf)
> + return ERR_PTR(-EINVAL);
> +
> + env = kzalloc(sizeof(*env), GFP_KERNEL | __GFP_NOWARN);
> + if (!env)
> + return ERR_PTR(-ENOMEM);
> +
> + log = &env->log;
> + log->level = BPF_LOG_KERNEL;
> +
> + btf = kzalloc(sizeof(*btf), GFP_KERNEL | __GFP_NOWARN);
> + if (!btf) {
> + err = -ENOMEM;
> + goto errout;
> + }
> + env->btf = btf;
> +
> + btf->base_btf = base_btf;
> + btf->start_id = base_btf->nr_types;
> + btf->start_str_off = base_btf->hdr.str_len;
> + btf->kernel_btf = true;
> + snprintf(btf->name, sizeof(btf->name), "%s", module_name);
> +
> + btf->data = kvmalloc(data_size, GFP_KERNEL 

Re: [RFC PATCH bpf-next 4/5] bpf: load and verify kernel module BTFs

[Greg Kroah-Hartman]

On Thu, Nov 05, 2020 at 08:39:25AM -0800, Jakub Kicinski wrote:
> On Wed, 4 Nov 2020 20:51:39 -0800 Andrii Nakryiko wrote:
> > Add kernel module listener that will load/validate and unload module BTF.
> > Module BTFs gets ID generated for them, which makes it possible to iterate
> > them with existing BTF iteration API. They are given their respective 
> > module's
> > names, which will get reported through GET_OBJ_INFO API. They are also 
> > marked
> > as in-kernel BTFs for tooling to distinguish them from user-provided BTFs.
> > 
> > Also, similarly to vmlinux BTF, kernel module BTFs are exposed through
> > sysfs as /sys/kernel/btf/. This is convenient for user-space
> > tools to inspect module BTF contents and dump their types with existing 
> > tools:
> 
> Is there any precedent for creating per-module files under a new
> sysfs directory structure? My intuition would be that these files 
> belong under /sys/module/

Ick, why?  What's wrong with them under btf?  The module core code
"owns" the /sys/modules/ tree.  If you want others to mess with that, it
will get tricky.


> Also the CC list on these patches is quite narrow. You should have 
> at least CCed the module maintainer. Adding some folks now.
> 
> > [vmuser@archvm bpf]$ ls -la /sys/kernel/btf
> > total 0
> > drwxr-xr-x  2 root root   0 Nov  4 19:46 .
> > drwxr-xr-x 13 root root   0 Nov  4 19:46 ..
> > 
> > ...
> > 
> > -r--r--r--  1 root root 888 Nov  4 19:46 irqbypass
> > -r--r--r--  1 root root  100225 Nov  4 19:46 kvm
> > -r--r--r--  1 root root   35401 Nov  4 19:46 kvm_intel
> > -r--r--r--  1 root root 120 Nov  4 19:46 pcspkr
> > -r--r--r--  1 root root 399 Nov  4 19:46 serio_raw
> > -r--r--r--  1 root root 4094095 Nov  4 19:46 vmlinux
> > 
> > Signed-off-by: Andrii Nakryiko 
> > ---
> >  include/linux/bpf.h|   2 +
> >  include/linux/module.h |   4 +
> >  kernel/bpf/btf.c   | 193 +
> >  kernel/bpf/sysfs_btf.c |   2 +-
> >  kernel/module.c|  32 +++
> >  5 files changed, 232 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index 2fffd30e13ac..3cb89cd7177b 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -36,9 +36,11 @@ struct seq_operations;
> >  struct bpf_iter_aux_info;
> >  struct bpf_local_storage;
> >  struct bpf_local_storage_map;
> > +struct kobject;
> >  
> >  extern struct idr btf_idr;
> >  extern spinlock_t btf_idr_lock;
> > +extern struct kobject *btf_kobj;

I don't see any Documentation/ABI/ updates for the sysfs changes here,
did I miss it?

thanks,

greg k-h

Re: [PATCH net-next v2] net/usb/r8153_ecm: support ECM mode for RTL8153

[Greg Kroah-Hartman]

On Mon, Nov 02, 2020 at 11:47:18AM -0800, Jakub Kicinski wrote:
> On Mon, 2 Nov 2020 07:20:15 + Hayes Wang wrote:
> > Jakub Kicinski 
> > > Can you describe the use case in more detail?
> > > 
> > > AFAICT r8152 defines a match for the exact same device.
> > > Does it not mean that which driver is used will be somewhat random
> > > if both are built?  
> > 
> > I export rtl_get_version() from r8152. It would return none zero
> > value if r8152 could support this device. Both r8152 and r8153_ecm
> > would check the return value of rtl_get_version() in porbe().
> > Therefore, if rtl_get_version() return none zero value, the r8152
> > is used for the device with vendor mode. Otherwise, the r8153_ecm
> > is used for the device with ECM mode.
> 
> Oh, I see, I missed that the rtl_get_version() checking is the inverse
> of r8152.
> 
> > > > +/* Define these values to match your device */
> > > > +#define VENDOR_ID_REALTEK  0x0bda
> > > > +#define VENDOR_ID_MICROSOFT0x045e
> > > > +#define VENDOR_ID_SAMSUNG  0x04e8
> > > > +#define VENDOR_ID_LENOVO   0x17ef
> > > > +#define VENDOR_ID_LINKSYS  0x13b1
> > > > +#define VENDOR_ID_NVIDIA   0x0955
> > > > +#define VENDOR_ID_TPLINK   0x2357  
> > > 
> > > $ git grep 0x2357 | grep -i tplink
> > > drivers/net/usb/cdc_ether.c:#define TPLINK_VENDOR_ID  0x2357
> > > drivers/net/usb/r8152.c:#define VENDOR_ID_TPLINK  0x2357
> > > drivers/usb/serial/option.c:#define TPLINK_VENDOR_ID  
> > > 0x2357
> > > 
> > > $ git grep 0x17ef | grep -i lenovo
> > > drivers/hid/hid-ids.h:#define USB_VENDOR_ID_LENOVO0x17ef
> > > drivers/hid/wacom.h:#define USB_VENDOR_ID_LENOVO  0x17ef
> > > drivers/net/usb/cdc_ether.c:#define LENOVO_VENDOR_ID  0x17ef
> > > drivers/net/usb/r8152.c:#define VENDOR_ID_LENOVO  0x17ef
> > > 
> > > Time to consolidate those vendor id defines perhaps?  
> > 
> > It seems that there is no such header file which I could include
> > or add the new vendor IDs.
> 
> Please create one. (Adding Greg KH to the recipients, in case there is
> a reason that USB subsystem doesn't have a common vendor id header.)

There is a reason, it's a nightmare to maintain and handle merges for,
just don't do it.

Read the comments at the top of the pci_ids.h file if you are curious
why we don't even do this for PCI device ids anymore for the past 10+
years.

So no, please do not create such a common file, it is not needed or a
good idea.

thanks,

greg k-h

Re: [PATCH net-next 2/4] net: mvpp2: add mvpp2_phylink_to_port() helper

[Greg Kroah-Hartman]

On Mon, Nov 02, 2020 at 06:38:54PM +0100, Marcin Wojtas wrote:
> Hi Greg and Sasha,
> 
> pt., 9 paź 2020 o 05:43 Marcin Wojtas  napisał(a):
> >
> > Hi,
> >
> > sob., 20 cze 2020 o 11:21 Russell King  
> > napisał(a):
> > >
> > > Add a helper to convert the struct phylink_config pointer passed in
> > > from phylink to the drivers internal struct mvpp2_port.
> > >
> > > Signed-off-by: Russell King 
> > > ---
> > >  .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 29 +--
> > >  1 file changed, 14 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c 
> > > b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > index 7653277d03b7..313f5a60a605 100644
> > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > @@ -4767,12 +4767,16 @@ static void mvpp2_port_copy_mac_addr(struct 
> > > net_device *dev, struct mvpp2 *priv,
> > > eth_hw_addr_random(dev);
> > >  }
> > >
> > > +static struct mvpp2_port *mvpp2_phylink_to_port(struct phylink_config 
> > > *config)
> > > +{
> > > +   return container_of(config, struct mvpp2_port, phylink_config);
> > > +}
> > > +
> > >  static void mvpp2_phylink_validate(struct phylink_config *config,
> > >unsigned long *supported,
> > >struct phylink_link_state *state)
> > >  {
> > > -   struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > > -  phylink_config);
> > > +   struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > > __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> > >
> > > /* Invalid combinations */
> > > @@ -4913,8 +4917,7 @@ static void mvpp2_gmac_pcs_get_state(struct 
> > > mvpp2_port *port,
> > >  static void mvpp2_phylink_mac_pcs_get_state(struct phylink_config 
> > > *config,
> > > struct phylink_link_state 
> > > *state)
> > >  {
> > > -   struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > > -  phylink_config);
> > > +   struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > >
> > > if (port->priv->hw_version == MVPP22 && port->gop_id == 0) {
> > > u32 mode = readl(port->base + MVPP22_XLG_CTRL3_REG);
> > > @@ -4931,8 +4934,7 @@ static void mvpp2_phylink_mac_pcs_get_state(struct 
> > > phylink_config *config,
> > >
> > >  static void mvpp2_mac_an_restart(struct phylink_config *config)
> > >  {
> > > -   struct mvpp2_port *port = container_of(config, struct mvpp2_port,
> > > -  phylink_config);
> > > +   struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > > u32 val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> > >
> > > writel(val | MVPP2_GMAC_IN_BAND_RESTART_AN,
> > > @@ -5105,13 +5107,12 @@ static void mvpp2_gmac_config(struct mvpp2_port 
> > > *port, unsigned int mode,
> > >  static void mvpp2_mac_config(struct phylink_config *config, unsigned int 
> > > mode,
> > >  const struct phylink_link_state *state)
> > >  {
> > > -   struct net_device *dev = to_net_dev(config->dev);
> > > -   struct mvpp2_port *port = netdev_priv(dev);
> > > +   struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > > bool change_interface = port->phy_interface != state->interface;
> > >
> > > /* Check for invalid configuration */
> > > if (mvpp2_is_xlg(state->interface) && port->gop_id != 0) {
> > > -   netdev_err(dev, "Invalid mode on %s\n", dev->name);
> > > +   netdev_err(port->dev, "Invalid mode on %s\n", 
> > > port->dev->name);
> > > return;
> > > }
> > >
> > > @@ -5151,8 +5152,7 @@ static void mvpp2_mac_link_up(struct phylink_config 
> > > *config,
> > >   int speed, int duplex,
> > >   bool tx_pause, bool rx_pause)
> > >  {
> > > -   struct net_device *dev = to_net_dev(config->dev);
> > > -   struct mvpp2_port *port = netdev_priv(dev);
> > > +   struct mvpp2_port *port = mvpp2_phylink_to_port(config);
> > > u32 val;
> > >
> > > if (mvpp2_is_xlg(interface)) {
> > > @@ -5199,14 +5199,13 @@ static void mvpp2_mac_link_up(struct 
> > > phylink_config *config,
> > >
> > > mvpp2_egress_enable(port);
> > > mvpp2_ingress_enable(port);
> > > -   netif_tx_wake_all_queues(dev);
> > > +   netif_tx_wake_all_queues(port->dev);
> > >  }
> > >
> > >  static void mvpp2_mac_link_down(struct phylink_config *config,
> > > unsigned int mode, phy_interface_t 
> > > interface)
> > >  {
> > > -   struct net_device *dev = to_net_dev(config->dev);
> > > -   struct mvpp2_port *port = netdev_priv(dev);
> > > +   struct m

Re: [PATCH v2 20/39] docs: ABI: testing: make the files compatible with ReST output

[Greg Kroah-Hartman]

On Mon, Nov 02, 2020 at 12:04:36PM +0100, Fabrice Gasnier wrote:
> On 10/30/20 11:09 AM, Mauro Carvalho Chehab wrote:
> > Em Fri, 30 Oct 2020 10:19:12 +0100
> > Fabrice Gasnier  escreveu:
> > 
> >> Hi Mauro,
> >>
> >> [...]
> >>
> >>>  
> >>> +What:
> >>> /sys/bus/iio/devices/iio:deviceX/in_count_quadrature_mode_available
> >>> +KernelVersion:   4.12
> >>> +Contact: benjamin.gaign...@st.com
> >>> +Description:
> >>> + Reading returns the list possible quadrature modes.
> >>> +
> >>> +What:
> >>> /sys/bus/iio/devices/iio:deviceX/in_count0_quadrature_mode
> >>> +KernelVersion:   4.12
> >>> +Contact: benjamin.gaign...@st.com
> >>> +Description:
> >>> + Configure the device counter quadrature modes:
> >>> +
> >>> + channel_A:
> >>> + Encoder A input servers as the count input and B as
> >>> + the UP/DOWN direction control input.
> >>> +
> >>> + channel_B:
> >>> + Encoder B input serves as the count input and A as
> >>> + the UP/DOWN direction control input.
> >>> +
> >>> + quadrature:
> >>> + Encoder A and B inputs are mixed to get direction
> >>> + and count with a scale of 0.25.
> >>> +  
> >>
> > 
> > Hi Fabrice,
> > 
> >> I just noticed that since Jonathan question in v1.
> >>
> >> Above ABI has been moved in the past as discussed in [1]. You can take a
> >> look at:
> >> b299d00 IIO: stm32: Remove quadrature related functions from trigger driver
> >>
> >> Could you please remove the above chunk ?
> >>
> >> With that, for the stm32 part:
> >> Acked-by: Fabrice Gasnier 
> > 
> > 
> > Hmm... probably those were re-introduced due to a rebase. This
> > series were originally written about 1,5 years ago.
> > 
> > I'll drop those hunks.
> 
> Hi Mauro, Greg,
> 
> I just figured out this patch has been applied with above hunk.
> 
> This should be dropped: is there a fix on its way already ?
> (I may have missed it)

Can you send a fix for just this hunk?

thanks,

greg k-h

Re: [RFC] wimax: move out to staging

[Greg Kroah-Hartman]

On Tue, Oct 27, 2020 at 10:20:13PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann 
> 
> There are no known users of this driver as of October 2020, and it will
> be removed unless someone turns out to still need it in future releases.
> 
> According to https://en.wikipedia.org/wiki/List_of_WiMAX_networks, there
> have been many public wimax networks, but it appears that these entries
> are all stale, after everyone has migrated to LTE or discontinued their
> service altogether.
> 
> NetworkManager appears to have dropped userspace support in 2015
> https://bugzilla.gnome.org/show_bug.cgi?id=747846, the
> www.linuxwimax.org
> site had already shut down earlier.
> 
> WiMax is apparently still being deployed on airport campus networks
> ("AeroMACS"), but in a frequency band that was not supported by the old
> Intel 2400m (used in Sandy Bridge laptops and earlier), which is the
> only driver using the kernel's wimax stack.
> 
> Move all files into drivers/staging/wimax, including the uapi header
> files and documentation, to make it easier to remove it when it gets
> to that. Only minimal changes are made to the source files, in order
> to make it possible to port patches across the move.
> 
> Also remove the MAINTAINERS entry that refers to a broken mailing
> list and website.
> 
> Suggested-by: Inaky Perez-Gonzalez 
> Signed-off-by: Arnd Bergmann 

Is this ok for me to take through the staging tree?  If so, I need an
ack from the networking maintainers.

If not, feel free to send it through the networking tree and add:

Acked-by: Greg Kroah-Hartman 

[PATCH 5.9 457/757] perf metricgroup: Fix uncore metric expressions

[Greg Kroah-Hartman]

From: Ian Rogers 

[ Upstream commit dcc81be0fc4e66943041e6e19a5faf8f8704a27e ]

A metric like DRAM_BW_Use has on SkylakeX events uncore_imc/cas_count_read/
and uncore_imc/case_count_write/.

These events open 6 events per socket with pmu names of
uncore_imc_[0-5].

The current metric setup code in find_evsel_group assumes one ID will
map to 1 event to be recorded in metric_events.

For events with multiple matches, the first event is recorded in
metric_events (avoiding matching >1 event with the same name) and the
evlist_used updated so that duplicate events aren't removed when the
evlist has unused events removed.

Before this change:

  $ /tmp/perf/perf stat -M DRAM_BW_Use -a -- sleep 1

   Performance counter stats for 'system wide':

   41.14 MiB  uncore_imc/cas_count_read/
   1,002,614,251 ns   duration_time

 1.002614251 seconds time elapsed

After this change:

  $ /tmp/perf/perf stat -M DRAM_BW_Use -a -- sleep 1

   Performance counter stats for 'system wide':

  157.47 MiB  uncore_imc/cas_count_read/ # 0.00 DRAM_BW_Use
  126.97 MiB  uncore_imc/cas_count_write/
   1,003,019,728 ns   duration_time

Erroneous duplication introduced in:
commit 2440689d62e9 ("perf metricgroup: Remove duped metric group events").

Fixes: ded80bda8bc9 ("perf expr: Migrate expr ids table to a hashmap").
Reported-by: Jin Yao 
Signed-off-by: Ian Rogers 
Cc: Adrian Hunter 
Cc: Alexander Shishkin 
Cc: Alexei Starovoitov 
Cc: Andi Kleen 
Cc: Andrii Nakryiko 
Cc: Athira Jajeev 
Cc: Daniel Borkmann 
Cc: Jiri Olsa 
Cc: John Fastabend 
Cc: KP Singh 
Cc: Mark Rutland 
Cc: Martin KaFai Lau 
Cc: Namhyung Kim 
Cc: Peter Zijlstra 
Cc: Song Liu 
Cc: Stephane Eranian 
Cc: Yonghong Song 
Cc: b...@vger.kernel.org
Cc: netdev@vger.kernel.org
Link: http://lore.kernel.org/lkml/20200917201807.4090224-1-irog...@google.com
Signed-off-by: Arnaldo Carvalho de Melo 
Signed-off-by: Sasha Levin 
---
 tools/perf/util/metricgroup.c | 75 ++-
 1 file changed, 56 insertions(+), 19 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index ab5030fcfed4e..d948a7f910cfa 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -150,6 +150,18 @@ static void expr_ids__exit(struct expr_ids *ids)
free(ids->id[i].id);
 }
 
+static bool contains_event(struct evsel **metric_events, int num_events,
+   const char *event_name)
+{
+   int i;
+
+   for (i = 0; i < num_events; i++) {
+   if (!strcmp(metric_events[i]->name, event_name))
+   return true;
+   }
+   return false;
+}
+
 /**
  * Find a group of events in perf_evlist that correpond to those from a parsed
  * metric expression. Note, as find_evsel_group is called in the same order as
@@ -180,7 +192,11 @@ static struct evsel *find_evsel_group(struct evlist 
*perf_evlist,
int i = 0, matched_events = 0, events_to_match;
const int idnum = (int)hashmap__size(&pctx->ids);
 
-   /* duration_time is grouped separately. */
+   /*
+* duration_time is always grouped separately, when events are grouped
+* (ie has_constraint is false) then ignore it in the matching loop and
+* add it to metric_events at the end.
+*/
if (!has_constraint &&
hashmap__find(&pctx->ids, "duration_time", (void **)&val_ptr))
events_to_match = idnum - 1;
@@ -207,23 +223,20 @@ static struct evsel *find_evsel_group(struct evlist 
*perf_evlist,
sizeof(struct evsel *) * idnum);
current_leader = ev->leader;
}
-   if (hashmap__find(&pctx->ids, ev->name, (void **)&val_ptr)) {
-   if (has_constraint) {
-   /*
-* Events aren't grouped, ensure the same event
-* isn't matched from two groups.
-*/
-   for (i = 0; i < matched_events; i++) {
-   if (!strcmp(ev->name,
-   metric_events[i]->name)) {
-   break;
-   }
-   }
-   if (i != matched_events)
-   continue;
-   }
+   /*
+* Check for duplicate events with the same name. For example,
+* uncore_imc/cas_count_read/ will turn into 6 events per socket
+* on skylakex. Only the first such event is placed in
+* metric_events. If events aren't grouped then this also
+* ensures that the same event in different sibling groups
+* aren't both added to metric_events.
+ 

Re: [PATCH v2 net] staging: octeon: Drop on uncorrectable alignment or FCS error

[Greg Kroah-Hartman]

On Fri, Oct 16, 2020 at 12:18:58PM +0200, Alexander A Sverdlin wrote:
> From: Alexander Sverdlin 
> 
> Currently in case of alignment or FCS error if the packet cannot be
> corrected it's still not dropped. Report the error properly and drop the
> packet while making the code around a little bit more readable.
> 
> Signed-off-by: Alexander Sverdlin 
> Fixes: 80ff0fd3ab ("Staging: Add octeon-ethernet driver files.")
> 
> Change-Id: Ie1fadcc57cb5e221cf3e83c169b53a5533b8edff

Please stop using gerrit for patches destined for upstream development
:(

Re: [PATCH v2 net] staging: octeon: Drop on uncorrectable alignment or FCS error

[Greg Kroah-Hartman]

On Fri, Oct 16, 2020 at 12:18:58PM +0200, Alexander A Sverdlin wrote:
> From: Alexander Sverdlin 
> 
> Currently in case of alignment or FCS error if the packet cannot be
> corrected it's still not dropped. Report the error properly and drop the
> packet while making the code around a little bit more readable.
> 
> Signed-off-by: Alexander Sverdlin 
> Fixes: 80ff0fd3ab ("Staging: Add octeon-ethernet driver files.")
> 
> Change-Id: Ie1fadcc57cb5e221cf3e83c169b53a5533b8edff

Why is the change-id still here???

Re: [PATCH v2 net] staging: octeon: Drop on uncorrectable alignment or FCS error

[Greg Kroah-Hartman]

On Fri, Oct 16, 2020 at 12:18:58PM +0200, Alexander A Sverdlin wrote:
> From: Alexander Sverdlin 
> 
> Currently in case of alignment or FCS error if the packet cannot be
> corrected it's still not dropped. Report the error properly and drop the
> packet while making the code around a little bit more readable.
> 
> Signed-off-by: Alexander Sverdlin 
> Fixes: 80ff0fd3ab ("Staging: Add octeon-ethernet driver files.")
> 
> Change-Id: Ie1fadcc57cb5e221cf3e83c169b53a5533b8edff

You didn't run checkpatch :(

[PATCH 4.9 47/54] mdio: fix mdio-thunder.c dependency & build error

[Greg Kroah-Hartman]

From: Randy Dunlap 

[ Upstream commit 7dbbcf496f2a4b6d82cfc7810a0746e160b79762 ]

Fix build error by selecting MDIO_DEVRES for MDIO_THUNDER.
Fixes this build error:

ld: drivers/net/phy/mdio-thunder.o: in function `thunder_mdiobus_pci_probe':
drivers/net/phy/mdio-thunder.c:78: undefined reference to 
`devm_mdiobus_alloc_size'

Fixes: 379d7ac7ca31 ("phy: mdio-thunder: Add driver for Cavium Thunder SoC MDIO 
buses.")
Reported-by: kernel test robot 
Signed-off-by: Randy Dunlap 
Cc: Bartosz Golaszewski 
Cc: Andrew Lunn 
Cc: Heiner Kallweit 
Cc: netdev@vger.kernel.org
Cc: David Daney 
Reviewed-by: Andrew Lunn 
Signed-off-by: David S. Miller 
Signed-off-by: Sasha Levin 
---
 drivers/net/phy/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 2651c8d8de2f8..032017bd0ced5 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -135,6 +135,7 @@ config MDIO_THUNDER
depends on 64BIT
depends on PCI
select MDIO_CAVIUM
+   select MDIO_DEVRES
help
  This driver supports the MDIO interfaces found on Cavium
  ThunderX SoCs when the MDIO bus device appears as a PCI
-- 
2.25.1

[PATCH 4.14 61/70] mdio: fix mdio-thunder.c dependency & build error

[Greg Kroah-Hartman]

From: Randy Dunlap 

[ Upstream commit 7dbbcf496f2a4b6d82cfc7810a0746e160b79762 ]

Fix build error by selecting MDIO_DEVRES for MDIO_THUNDER.
Fixes this build error:

ld: drivers/net/phy/mdio-thunder.o: in function `thunder_mdiobus_pci_probe':
drivers/net/phy/mdio-thunder.c:78: undefined reference to 
`devm_mdiobus_alloc_size'

Fixes: 379d7ac7ca31 ("phy: mdio-thunder: Add driver for Cavium Thunder SoC MDIO 
buses.")
Reported-by: kernel test robot 
Signed-off-by: Randy Dunlap 
Cc: Bartosz Golaszewski 
Cc: Andrew Lunn 
Cc: Heiner Kallweit 
Cc: netdev@vger.kernel.org
Cc: David Daney 
Reviewed-by: Andrew Lunn 
Signed-off-by: David S. Miller 
Signed-off-by: Sasha Levin 
---
 drivers/net/phy/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index cd931cf9dcc26..e08d822338341 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -146,6 +146,7 @@ config MDIO_THUNDER
depends on 64BIT
depends on PCI
select MDIO_CAVIUM
+   select MDIO_DEVRES
help
  This driver supports the MDIO interfaces found on Cavium
  ThunderX SoCs when the MDIO bus device appears as a PCI
-- 
2.25.1

[PATCH 4.19 38/49] mdio: fix mdio-thunder.c dependency & build error

[Greg Kroah-Hartman]

From: Randy Dunlap 

[ Upstream commit 7dbbcf496f2a4b6d82cfc7810a0746e160b79762 ]

Fix build error by selecting MDIO_DEVRES for MDIO_THUNDER.
Fixes this build error:

ld: drivers/net/phy/mdio-thunder.o: in function `thunder_mdiobus_pci_probe':
drivers/net/phy/mdio-thunder.c:78: undefined reference to 
`devm_mdiobus_alloc_size'

Fixes: 379d7ac7ca31 ("phy: mdio-thunder: Add driver for Cavium Thunder SoC MDIO 
buses.")
Reported-by: kernel test robot 
Signed-off-by: Randy Dunlap 
Cc: Bartosz Golaszewski 
Cc: Andrew Lunn 
Cc: Heiner Kallweit 
Cc: netdev@vger.kernel.org
Cc: David Daney 
Reviewed-by: Andrew Lunn 
Signed-off-by: David S. Miller 
Signed-off-by: Sasha Levin 
---
 drivers/net/phy/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 1f5fd24cd749e..2386871e12949 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -154,6 +154,7 @@ config MDIO_THUNDER
depends on 64BIT
depends on PCI
select MDIO_CAVIUM
+   select MDIO_DEVRES
help
  This driver supports the MDIO interfaces found on Cavium
  ThunderX SoCs when the MDIO bus device appears as a PCI
-- 
2.25.1

[PATCH 5.8 073/124] mdio: fix mdio-thunder.c dependency & build error

[Greg Kroah-Hartman]

From: Randy Dunlap 

[ Upstream commit 7dbbcf496f2a4b6d82cfc7810a0746e160b79762 ]

Fix build error by selecting MDIO_DEVRES for MDIO_THUNDER.
Fixes this build error:

ld: drivers/net/phy/mdio-thunder.o: in function `thunder_mdiobus_pci_probe':
drivers/net/phy/mdio-thunder.c:78: undefined reference to 
`devm_mdiobus_alloc_size'

Fixes: 379d7ac7ca31 ("phy: mdio-thunder: Add driver for Cavium Thunder SoC MDIO 
buses.")
Reported-by: kernel test robot 
Signed-off-by: Randy Dunlap 
Cc: Bartosz Golaszewski 
Cc: Andrew Lunn 
Cc: Heiner Kallweit 
Cc: netdev@vger.kernel.org
Cc: David Daney 
Reviewed-by: Andrew Lunn 
Signed-off-by: David S. Miller 
Signed-off-by: Sasha Levin 
---
 drivers/net/phy/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index e351d65533aa8..06146ae4c6d8d 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -217,6 +217,7 @@ config MDIO_THUNDER
depends on 64BIT
depends on PCI
select MDIO_CAVIUM
+   select MDIO_DEVRES
help
  This driver supports the MDIO interfaces found on Cavium
  ThunderX SoCs when the MDIO bus device appears as a PCI
-- 
2.25.1

[PATCH 5.4 60/85] mdio: fix mdio-thunder.c dependency & build error

[Greg Kroah-Hartman]

From: Randy Dunlap 

[ Upstream commit 7dbbcf496f2a4b6d82cfc7810a0746e160b79762 ]

Fix build error by selecting MDIO_DEVRES for MDIO_THUNDER.
Fixes this build error:

ld: drivers/net/phy/mdio-thunder.o: in function `thunder_mdiobus_pci_probe':
drivers/net/phy/mdio-thunder.c:78: undefined reference to 
`devm_mdiobus_alloc_size'

Fixes: 379d7ac7ca31 ("phy: mdio-thunder: Add driver for Cavium Thunder SoC MDIO 
buses.")
Reported-by: kernel test robot 
Signed-off-by: Randy Dunlap 
Cc: Bartosz Golaszewski 
Cc: Andrew Lunn 
Cc: Heiner Kallweit 
Cc: netdev@vger.kernel.org
Cc: David Daney 
Reviewed-by: Andrew Lunn 
Signed-off-by: David S. Miller 
Signed-off-by: Sasha Levin 
---
 drivers/net/phy/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index fe602648b99f5..dcf2051ef2c04 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -193,6 +193,7 @@ config MDIO_THUNDER
depends on 64BIT
depends on PCI
select MDIO_CAVIUM
+   select MDIO_DEVRES
help
  This driver supports the MDIO interfaces found on Cavium
  ThunderX SoCs when the MDIO bus device appears as a PCI
-- 
2.25.1

Re: [PATCH 3/8] staging: wfx: standardize the error when vif does not exist

[Greg Kroah-Hartman]

On Sat, Oct 10, 2020 at 02:22:13PM +0200, Jérôme Pouiller wrote:
> On Friday 9 October 2020 20:52:47 CEST Kalle Valo wrote:
> > Jerome Pouiller  writes:
> > 
> > > From: Jérôme Pouiller 
> > >
> > > Smatch complains:
> > >
> > >drivers/staging/wfx/hif_rx.c:177 hif_scan_complete_indication() warn: 
> > > potential NULL parameter dereference 'wvif'
> > >drivers/staging/wfx/data_tx.c:576 wfx_flush() warn: potential NULL 
> > > parameter dereference 'wvif'
> > >
> > > Indeed, if the vif id returned by the device does not exist anymore,
> > > wdev_to_wvif() could return NULL.
> > >
> > > In add, the error is not handled uniformly in the code, sometime a
> > > WARN() is displayed but code continue, sometime a dev_warn() is
> > > displayed, sometime it is just not tested, ...
> > >
> > > This patch standardize that.
> > >
> > > Reported-by: Dan Carpenter 
> > > Signed-off-by: Jérôme Pouiller 
> > > ---
> > >  drivers/staging/wfx/data_tx.c |  5 -
> > >  drivers/staging/wfx/hif_rx.c  | 34 --
> > >  drivers/staging/wfx/sta.c |  4 
> > >  3 files changed, 32 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
> > > index b4d5dd3d2d23..8db0be08daf8 100644
> > > --- a/drivers/staging/wfx/data_tx.c
> > > +++ b/drivers/staging/wfx/data_tx.c
> > > @@ -431,7 +431,10 @@ static void wfx_skb_dtor(struct wfx_vif *wvif, 
> > > struct sk_buff *skb)
> > > sizeof(struct hif_req_tx) +
> > > req->fc_offset;
> > >
> > > - WARN_ON(!wvif);
> > > + if (!wvif) {
> > > + pr_warn("%s: vif associated with the skb does not exist 
> > > anymore\n", __func__);
> > > + return;
> > > + }
> > 
> > I'm not really a fan of using function names in warning or error
> > messages as it clutters the log. In debug messages I think they are ok.
> 
> In the initial code, I used WARN() that far more clutters the log (I
> have stated that a backtrace won't provide any useful information, so
> pr_warn() was better suited).
> 
> In add, in my mind, these warnings are debug messages. If they appears,
> the user should probably report a bug.
> 
> Finally, in this patch, I use the same message several times (ok, not
> this particular one). So the function name is a way to differentiate
> them.

You should use dev_*() for these, that way you can properly determine
the exact device as well.

thanks,

greg k-h

Re: [PATCH v2 4/4] dt-bindings: usb: use preferred license tag

[Greg Kroah-Hartman]

On Sat, Oct 10, 2020 at 04:43:14PM +0800, Chunfeng Yun wrote:
> This is used to fix the checkpach.pl WARNING:SPDX_LICENSE_TAG
> 
> See bindings/submitting-patches.rst:
> "DT binding files should be dual licensed. The preferred license tag is
>  (GPL-2.0-only OR BSD-2-Clause)."
> 
> Signed-off-by: Chunfeng Yun 
> ---
> v2: new patch
> ---
>  Documentation/devicetree/bindings/usb/usb-hcd.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/usb-hcd.yaml 
> b/Documentation/devicetree/bindings/usb/usb-hcd.yaml
> index 42b295afdf32..11b9b9ee2b54 100644
> --- a/Documentation/devicetree/bindings/usb/usb-hcd.yaml
> +++ b/Documentation/devicetree/bindings/usb/usb-hcd.yaml
> @@ -1,4 +1,4 @@
> -# SPDX-License-Identifier: GPL-2.0
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

Are you sure you are allowed to change the license of this file?  Last I
checked, you did not write this file, and so, you can't change the
license of it.  You need to get the owners of the file to do so.

thanks,

greg k-h

Re: [PATCH] staging: octeon: repair "fixed-link" support

[Greg Kroah-Hartman]

On Fri, Oct 09, 2020 at 11:47:39AM +0200, Alexander A Sverdlin wrote:
> From: Alexander Sverdlin 
> 
> The PHYs must be registered once in device probe function, not in device
> open callback because it's only possible to register them once.
> 
> Fixes: a25e278020 ("staging: octeon: support fixed-link phys")
> Signed-off-by: Alexander Sverdlin 

Looks like it breaks the build, please fix up and test your patches when
sending them out next time.

thanks,

greg k-h

Re: [PATCH] staging: octeon: repair "fixed-link" support

[Greg Kroah-Hartman]

On Fri, Oct 09, 2020 at 11:40:24AM +0200, Alexander Sverdlin wrote:
> Hello Greg, Dave and all,
> 
> the below patch is still applicable as-is, would you please re-consider it 
> now,
> as the driver has been undeleted?
> 
> On 08/01/2020 17:09, Alexander X Sverdlin wrote:

Why would we have a patch from January still in our inboxes? :)

Please resend.

thanks,

greg k-h

Re: [PATCH] staging: octeon: Drop on uncorrectable alignment or FCS error

[Greg Kroah-Hartman]

On Fri, Oct 09, 2020 at 11:34:47AM +0200, Alexander Sverdlin wrote:
> Hello Greg,
> 
> On 10/01/2020 13:48, Greg Kroah-Hartman wrote:
> > On Wed, Jan 08, 2020 at 05:10:42PM +0100, Alexander X Sverdlin wrote:
> >> From: Alexander Sverdlin 
> >>
> >> Currently in case of alignment or FCS error if the packet cannot be
> >> corrected it's still not dropped. Report the error properly and drop the
> >> packet while making the code around a little bit more readable.
> >>
> >> Fixes: 80ff0fd3ab ("Staging: Add octeon-ethernet driver files.")
> >> Signed-off-by: Alexander Sverdlin 
> >> ---
> >>  drivers/staging/octeon/ethernet-rx.c | 18 +-
> >>  1 file changed, 9 insertions(+), 9 deletions(-)
> > 
> > This driver is now deleted from the tree, sorry.
> 
> Now that the driver is restored, would you please consider this patch again?

Feel free to submit it again if you feel it is still needed.

thanks,

greg k-h

Re: [PATCH] Revert "Bluetooth: Update resolving list when updating whitelist"

[Greg Kroah-Hartman]

On Wed, Oct 07, 2020 at 03:40:25PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Oct 07, 2020 at 03:23:21PM +0200, Greg Kroah-Hartman wrote:
> > On Mon, Oct 05, 2020 at 08:58:33PM +0200, Marcel Holtmann wrote:
> > > Hi Greg,
> > > 
> > > >>>>>>>>>>>>>> This reverts commit 
> > > >>>>>>>>>>>>>> 0eee35bdfa3b472cc986ecc6ad76293fdcda59e2 as it
> > > >>>>>>>>>>>>>> breaks all bluetooth connections on my machine.
> > > >>>>>>>>>>>>>> 
> > > >>>>>>>>>>>>>> Cc: Marcel Holtmann 
> > > >>>>>>>>>>>>>> Cc: Sathish Narsimman 
> > > >>>>>>>>>>>>>> Fixes: 0eee35bdfa3b ("Bluetooth: Update resolving list 
> > > >>>>>>>>>>>>>> when updating whitelist")
> > > >>>>>>>>>>>>>> Signed-off-by: Greg Kroah-Hartman 
> > > >>>>>>>>>>>>>> 
> > > >>>>>>>>>>>>>> ---
> > > >>>>>>>>>>>>>> net/bluetooth/hci_request.c | 41 
> > > >>>>>>>>>>>>>> ++---
> > > >>>>>>>>>>>>>> 1 file changed, 2 insertions(+), 39 deletions(-)
> > > >>>>>>>>>>>>>> 
> > > >>>>>>>>>>>>>> This has been bugging me for since 5.9-rc1, when all 
> > > >>>>>>>>>>>>>> bluetooth devices
> > > >>>>>>>>>>>>>> stopped working on my desktop system.  I finally got the 
> > > >>>>>>>>>>>>>> time to do
> > > >>>>>>>>>>>>>> bisection today, and it came down to this patch.  
> > > >>>>>>>>>>>>>> Reverting it on top of
> > > >>>>>>>>>>>>>> 5.9-rc7 restored bluetooth devices and now my input 
> > > >>>>>>>>>>>>>> devices properly
> > > >>>>>>>>>>>>>> work.
> > > >>>>>>>>>>>>>> 
> > > >>>>>>>>>>>>>> As it's almost 5.9-final, any chance this can be merged 
> > > >>>>>>>>>>>>>> now to fix the
> > > >>>>>>>>>>>>>> issue?
> > > >>>>>>>>>>>>> 
> > > >>>>>>>>>>>>> can you be specific what breaks since our guys and I also 
> > > >>>>>>>>>>>>> think the
> > > >>>>>>>>>>>>> ChromeOS guys have been testing these series of patches 
> > > >>>>>>>>>>>>> heavily.
> > > >>>>>>>>>>>> 
> > > >>>>>>>>>>>> My bluetooth trackball does not connect at all.  With this 
> > > >>>>>>>>>>>> reverted, it
> > > >>>>>>>>>>>> all "just works".
> > > >>>>>>>>>>>> 
> > > >>>>>>>>>>>> Same I think for a Bluetooth headset, can check that again 
> > > >>>>>>>>>>>> if you really
> > > >>>>>>>>>>>> need me to, but the trackball is reliable here.
> > > >>>>>>>>>>>> 
> > > >>>>>>>>>>>>> When you run btmon does it indicate any errors?
> > > >>>>>>>>>>>> 
> > > >>>>>>>>>>>> How do I run it and where are the errors displayed?
> > > >>>>>>>>>>> 
> > > >>>>>>>>>>> you can do btmon -w trace.log and just let it run like 
> > > >>>>>>>>>>> tcdpump.
> > > >>>>>>>>>> 
> > > >>>>>>>>>&g

Re: [PATCH] Revert "Bluetooth: Update resolving list when updating whitelist"

[Greg Kroah-Hartman]

On Wed, Oct 07, 2020 at 03:23:21PM +0200, Greg Kroah-Hartman wrote:
> On Mon, Oct 05, 2020 at 08:58:33PM +0200, Marcel Holtmann wrote:
> > Hi Greg,
> > 
> > >>>>>>>>>>>>>> This reverts commit 0eee35bdfa3b472cc986ecc6ad76293fdcda59e2 
> > >>>>>>>>>>>>>> as it
> > >>>>>>>>>>>>>> breaks all bluetooth connections on my machine.
> > >>>>>>>>>>>>>> 
> > >>>>>>>>>>>>>> Cc: Marcel Holtmann 
> > >>>>>>>>>>>>>> Cc: Sathish Narsimman 
> > >>>>>>>>>>>>>> Fixes: 0eee35bdfa3b ("Bluetooth: Update resolving list when 
> > >>>>>>>>>>>>>> updating whitelist")
> > >>>>>>>>>>>>>> Signed-off-by: Greg Kroah-Hartman 
> > >>>>>>>>>>>>>> 
> > >>>>>>>>>>>>>> ---
> > >>>>>>>>>>>>>> net/bluetooth/hci_request.c | 41 
> > >>>>>>>>>>>>>> ++---
> > >>>>>>>>>>>>>> 1 file changed, 2 insertions(+), 39 deletions(-)
> > >>>>>>>>>>>>>> 
> > >>>>>>>>>>>>>> This has been bugging me for since 5.9-rc1, when all 
> > >>>>>>>>>>>>>> bluetooth devices
> > >>>>>>>>>>>>>> stopped working on my desktop system.  I finally got the 
> > >>>>>>>>>>>>>> time to do
> > >>>>>>>>>>>>>> bisection today, and it came down to this patch.  Reverting 
> > >>>>>>>>>>>>>> it on top of
> > >>>>>>>>>>>>>> 5.9-rc7 restored bluetooth devices and now my input devices 
> > >>>>>>>>>>>>>> properly
> > >>>>>>>>>>>>>> work.
> > >>>>>>>>>>>>>> 
> > >>>>>>>>>>>>>> As it's almost 5.9-final, any chance this can be merged now 
> > >>>>>>>>>>>>>> to fix the
> > >>>>>>>>>>>>>> issue?
> > >>>>>>>>>>>>> 
> > >>>>>>>>>>>>> can you be specific what breaks since our guys and I also 
> > >>>>>>>>>>>>> think the
> > >>>>>>>>>>>>> ChromeOS guys have been testing these series of patches 
> > >>>>>>>>>>>>> heavily.
> > >>>>>>>>>>>> 
> > >>>>>>>>>>>> My bluetooth trackball does not connect at all.  With this 
> > >>>>>>>>>>>> reverted, it
> > >>>>>>>>>>>> all "just works".
> > >>>>>>>>>>>> 
> > >>>>>>>>>>>> Same I think for a Bluetooth headset, can check that again if 
> > >>>>>>>>>>>> you really
> > >>>>>>>>>>>> need me to, but the trackball is reliable here.
> > >>>>>>>>>>>> 
> > >>>>>>>>>>>>> When you run btmon does it indicate any errors?
> > >>>>>>>>>>>> 
> > >>>>>>>>>>>> How do I run it and where are the errors displayed?
> > >>>>>>>>>>> 
> > >>>>>>>>>>> you can do btmon -w trace.log and just let it run like tcdpump.
> > >>>>>>>>>> 
> > >>>>>>>>>> Ok, attached.
> > >>>>>>>>>> 
> > >>>>>>>>>> The device is not connecting, and then I open the gnome 
> > >>>>>>>>>> bluetooth dialog
> > >>>>>>>>>> and it scans for devices in the area, but does not connect to my
> > >>>>>>&

Re: [PATCH] Revert "Bluetooth: Update resolving list when updating whitelist"

[Greg Kroah-Hartman]

On Mon, Oct 05, 2020 at 08:58:33PM +0200, Marcel Holtmann wrote:
> Hi Greg,
> 
> >>>>>>>>>>>>>> This reverts commit 0eee35bdfa3b472cc986ecc6ad76293fdcda59e2 
> >>>>>>>>>>>>>> as it
> >>>>>>>>>>>>>> breaks all bluetooth connections on my machine.
> >>>>>>>>>>>>>> 
> >>>>>>>>>>>>>> Cc: Marcel Holtmann 
> >>>>>>>>>>>>>> Cc: Sathish Narsimman 
> >>>>>>>>>>>>>> Fixes: 0eee35bdfa3b ("Bluetooth: Update resolving list when 
> >>>>>>>>>>>>>> updating whitelist")
> >>>>>>>>>>>>>> Signed-off-by: Greg Kroah-Hartman 
> >>>>>>>>>>>>>> ---
> >>>>>>>>>>>>>> net/bluetooth/hci_request.c | 41 
> >>>>>>>>>>>>>> ++---
> >>>>>>>>>>>>>> 1 file changed, 2 insertions(+), 39 deletions(-)
> >>>>>>>>>>>>>> 
> >>>>>>>>>>>>>> This has been bugging me for since 5.9-rc1, when all bluetooth 
> >>>>>>>>>>>>>> devices
> >>>>>>>>>>>>>> stopped working on my desktop system.  I finally got the time 
> >>>>>>>>>>>>>> to do
> >>>>>>>>>>>>>> bisection today, and it came down to this patch.  Reverting it 
> >>>>>>>>>>>>>> on top of
> >>>>>>>>>>>>>> 5.9-rc7 restored bluetooth devices and now my input devices 
> >>>>>>>>>>>>>> properly
> >>>>>>>>>>>>>> work.
> >>>>>>>>>>>>>> 
> >>>>>>>>>>>>>> As it's almost 5.9-final, any chance this can be merged now to 
> >>>>>>>>>>>>>> fix the
> >>>>>>>>>>>>>> issue?
> >>>>>>>>>>>>> 
> >>>>>>>>>>>>> can you be specific what breaks since our guys and I also think 
> >>>>>>>>>>>>> the
> >>>>>>>>>>>>> ChromeOS guys have been testing these series of patches heavily.
> >>>>>>>>>>>> 
> >>>>>>>>>>>> My bluetooth trackball does not connect at all.  With this 
> >>>>>>>>>>>> reverted, it
> >>>>>>>>>>>> all "just works".
> >>>>>>>>>>>> 
> >>>>>>>>>>>> Same I think for a Bluetooth headset, can check that again if 
> >>>>>>>>>>>> you really
> >>>>>>>>>>>> need me to, but the trackball is reliable here.
> >>>>>>>>>>>> 
> >>>>>>>>>>>>> When you run btmon does it indicate any errors?
> >>>>>>>>>>>> 
> >>>>>>>>>>>> How do I run it and where are the errors displayed?
> >>>>>>>>>>> 
> >>>>>>>>>>> you can do btmon -w trace.log and just let it run like tcdpump.
> >>>>>>>>>> 
> >>>>>>>>>> Ok, attached.
> >>>>>>>>>> 
> >>>>>>>>>> The device is not connecting, and then I open the gnome bluetooth 
> >>>>>>>>>> dialog
> >>>>>>>>>> and it scans for devices in the area, but does not connect to my
> >>>>>>>>>> existing devices at all.
> >>>>>>>>>> 
> >>>>>>>>>> Any ideas?
> >>>>>>>>> 
> >>>>>>>>> the trace file is from -rc7 or from -rc7 with this patch reverted?
> >>>>>>>>> 
> >>>>>>>>> I asked, because I see no hint that anything goes wrong. However I 
> >>>>>

Re: [PATCH 0/7] wfx: move out from the staging area

[Greg Kroah-Hartman]

On Wed, Oct 07, 2020 at 12:19:36PM +0200, Jerome Pouiller wrote:
> From: Jérôme Pouiller 
> 
> I think the wfx driver is now mature enough to be accepted in the
> drivers/net/wireless directory.
> 
> There is still one item on the TODO list. It is an idea to improve the rate
> control in some particular cases[1]. However, the current performances of the
> driver seem to satisfy everyone. In add, the suggested change is large enough.
> So, I would prefer to implement it only if it really solves an issue. I think 
> it
> is not an obstacle to move the driver out of the staging area.
> 
> In order to comply with the last rules for the DT bindings, I have converted 
> the
> documentation to yaml. I am moderately happy with the result. Especially, for
> the description of the binding. Any comments are welcome.
> 
> The series also update the copyrights dates of the files. I don't know exactly
> how this kind of changes should be sent. It's a bit weird to change all the
> copyrights in one commit, but I do not see any better way.
> 
> I also include a few fixes I have found these last weeks.
> 
> [1] https://lore.kernel.org/lkml/3099559.gv3Q75KnN1@pc-42

I'll take the first 6 patches here, the last one you should work with
the wireless maintainers to get reviewed.

Maybe that might want to wait until after 5.10-rc1 is out, with all of
these changes in it, making it an easier move.

thanks,

greg k-h

Re: [PATCH] Revert "Bluetooth: Update resolving list when updating whitelist"

[Greg Kroah-Hartman]

On Mon, Oct 05, 2020 at 07:38:35PM +0200, Greg Kroah-Hartman wrote:
> On Mon, Oct 05, 2020 at 07:14:44PM +0200, Marcel Holtmann wrote:
> > Hi Greg,
> > 
> > >>>>>>>>>>> This reverts commit 0eee35bdfa3b472cc986ecc6ad76293fdcda59e2 as 
> > >>>>>>>>>>> it
> > >>>>>>>>>>> breaks all bluetooth connections on my machine.
> > >>>>>>>>>>> 
> > >>>>>>>>>>> Cc: Marcel Holtmann 
> > >>>>>>>>>>> Cc: Sathish Narsimman 
> > >>>>>>>>>>> Fixes: 0eee35bdfa3b ("Bluetooth: Update resolving list when 
> > >>>>>>>>>>> updating whitelist")
> > >>>>>>>>>>> Signed-off-by: Greg Kroah-Hartman 
> > >>>>>>>>>>> ---
> > >>>>>>>>>>> net/bluetooth/hci_request.c | 41 
> > >>>>>>>>>>> ++---
> > >>>>>>>>>>> 1 file changed, 2 insertions(+), 39 deletions(-)
> > >>>>>>>>>>> 
> > >>>>>>>>>>> This has been bugging me for since 5.9-rc1, when all bluetooth 
> > >>>>>>>>>>> devices
> > >>>>>>>>>>> stopped working on my desktop system.  I finally got the time 
> > >>>>>>>>>>> to do
> > >>>>>>>>>>> bisection today, and it came down to this patch.  Reverting it 
> > >>>>>>>>>>> on top of
> > >>>>>>>>>>> 5.9-rc7 restored bluetooth devices and now my input devices 
> > >>>>>>>>>>> properly
> > >>>>>>>>>>> work.
> > >>>>>>>>>>> 
> > >>>>>>>>>>> As it's almost 5.9-final, any chance this can be merged now to 
> > >>>>>>>>>>> fix the
> > >>>>>>>>>>> issue?
> > >>>>>>>>>> 
> > >>>>>>>>>> can you be specific what breaks since our guys and I also think 
> > >>>>>>>>>> the
> > >>>>>>>>>> ChromeOS guys have been testing these series of patches heavily.
> > >>>>>>>>> 
> > >>>>>>>>> My bluetooth trackball does not connect at all.  With this 
> > >>>>>>>>> reverted, it
> > >>>>>>>>> all "just works".
> > >>>>>>>>> 
> > >>>>>>>>> Same I think for a Bluetooth headset, can check that again if you 
> > >>>>>>>>> really
> > >>>>>>>>> need me to, but the trackball is reliable here.
> > >>>>>>>>> 
> > >>>>>>>>>> When you run btmon does it indicate any errors?
> > >>>>>>>>> 
> > >>>>>>>>> How do I run it and where are the errors displayed?
> > >>>>>>>> 
> > >>>>>>>> you can do btmon -w trace.log and just let it run like tcdpump.
> > >>>>>>> 
> > >>>>>>> Ok, attached.
> > >>>>>>> 
> > >>>>>>> The device is not connecting, and then I open the gnome bluetooth 
> > >>>>>>> dialog
> > >>>>>>> and it scans for devices in the area, but does not connect to my
> > >>>>>>> existing devices at all.
> > >>>>>>> 
> > >>>>>>> Any ideas?
> > >>>>>> 
> > >>>>>> the trace file is from -rc7 or from -rc7 with this patch reverted?
> > >>>>>> 
> > >>>>>> I asked, because I see no hint that anything goes wrong. However I 
> > >>>>>> have a suspicion if you bisected it to this patch.
> > >>>>>> 
> > >>>>>> diff --git a/net/bluetooth/hci_request.c 
> > >>>>>> b/net/bluetooth/hci_request.c
> > >>>>>> index e0269192f2e5..94c0daa9f28d 100644
> >