* RT and PTP system timestamping
From: Vladimir Oltean @ 2019-09-01 19:02 UTC (permalink / raw)
To: netdev, linux-rt-users, linux-spi
Cc: Richard Cochran, Miroslav Lichvar, Hubert Feurstein
Hello people of netdev, linux-rt-users and linux-spi,
Apologies in advance for asking a question about something I know nothing about.
I am playing with a device driver of a SPI-controlled PTP timer. For
my particular application it is important that the value of the PTP
timer can be retrieved with an accuracy bound of less than +/- 400 ns.
Currently that job has been served by the PTP_SYS_OFFSET_EXTENDED [1]
ioctl (which allows for the timer's and the system's time to be
correlated) plus some hacks in the SPI core and drivers which were
submitted for review [2].
In the future I would like to evaluate RT on the device I am playing
with. There are a few dependency patches in flight and I'm not
actually clear what my current evaluation options as of now are, so I
think I'll just have to wait until 5.4-rt.
But at least I need to consider the RT friendliness of the solution I
am proposing. The gist of it (in the current version) is: put the
controller in poll mode, disable local IRQs and preemption on the
local CPU (via a spin_lock_irqsave), then surround the transfer of the
SPI byte I'm interested in with (basically) calls to
ktime_get_real_ts64.
Of course, this approach is completely incompatible with RT:
- In RT, spin_lock_irqsave does not disable IRQs and preemption. But
without those disabled, the PTP system timestamps are basically
throw-away - they should capture the most precise "before" and "after"
time of when the SPI slave device has received byte N of the transfer
(that is when it's snapshotting its timer internally). It's true that
the PTP_SYS_OFFSET_EXTENDED ioctl has a n_samples argument which can
be used in a sort of "pick shortest readout time" fashion, but there
is no guarantee that there will always be even 1 out of X readouts
that complete atomically with no preemption.
- Forcing the disabling of interrupts and preemption creates the
opportunity for unbounded latency for the other threads in the system.
- Disabling interrupts to record the before-and-after time for a TX
event would also be a contradiction in terms for SPI controllers that
don't do polling, i.e. DMA-based.
- In RT, there is no hardirq context at all given to a SPI controller driver.
The above constraints mean that the problem of timestamping byte N of
a SPI transfer is intractable from within the SPI controller driver
itself, at least with RT, as far as I can tell.
Ensuring the atomicness of one system clock readout with the SPI
transfer is hard enough I think, doing it for both the "pre" and the
"post" times is even more so. There have been discussions about only
taking one of the 2 timestamps, and deducing the other as being a
constant offset far. That is something I may be open to experiment
with, as long as there's least one place which can be timestamped,
that has a known offset relative to the hardware event (e.g. the
hardirq context - then the "pre" time can be backtraced).
I noticed the record_irq_time() function call in kernel/irq/handle.c
[3] and I do wonder whether it can be used for driver consumption?
I don't love the idea of moving the driver back to interrupt mode
though, but it's the only way I see currently. The other reason why I
put it in poll mode is that 50% of the time spent for a transfer is
simply wasted doing other stuff (an IRQ gets raised after each
transferred byte). This is secondary however.
I am looking forward to comments that will hopefully put me on the right path.
Regards,
-Vladimir
[1]: https://www.spinics.net/lists/netdev/msg532765.html
[2]: https://www.spinics.net/lists/netdev/msg593404.html
[3]: https://lwn.net/Articles/691297/
^ permalink raw reply
* Re: [PATCH v3 1/2] net: core: Notify on changes to dev->promiscuity.
From: Andrew Lunn @ 2019-09-01 18:48 UTC (permalink / raw)
To: Ido Schimmel
Cc: David Miller, jiri, horatiu.vultur, alexandre.belloni,
UNGLinuxDriver, allan.nielsen, ivecera, f.fainelli, netdev,
linux-kernel
In-Reply-To: <20190831204705.GA28380@splinter>
On Sat, Aug 31, 2019 at 11:47:05PM +0300, Ido Schimmel wrote:
> On Sat, Aug 31, 2019 at 09:35:56PM +0200, Andrew Lunn wrote:
> > > Also, what happens when I'm running these application without putting
> > > the interface in promisc mode? On an offloaded interface I would not be
> > > able to even capture packets addressed to my interface's MAC address.
> >
> > Sorry for rejoining the discussion late. I've been travelling and i'm
> > now 3/4 of the way to Lisbon.
>
> Hi Andrew,
>
> Have fun!
>
> > That statement i don't get.
>
> What about the other statements?
>
> > If the frame has the MAC address of the interface, it has to be
> > delivered to the CPU.
>
> So every packet that needs to be routed should be delivered to the CPU?
> Definitely not.
>
> > And so pcap will see it when running on the interface. I can pretty
> > much guarantee every DSA driver does that.
>
> I assume because you currently only consider L2 forwarding.
Yes, that is what i missed. The vast majority of switches which Linux
supports are L2. All the switches i deal with are L2. So i did not
think about L3. My bad.
> > But to address the bigger picture. My understanding is that we want to
> > model offloading as a mechanism to accelerate what Linux can already
> > do. The user should not have to care about these accelerators. The
> > interface should work like a normal Linux interface. I can put an IP
> > address on it and ping a peer. I can run a dhcp client and get an IP
> > address from a dhcp server. I can add the interface to a bridge, and
> > packets will get bridged. I as a user should not need to care if this
> > is done in software, or accelerated by offloading it. I can add a
> > route, and if the accelerate knows about L3, it can accelerate that as
> > well. If not, the kernel will route it.
>
> Yep, and this is how it's all working today.
So for a L3 switch, frames which match the MAC address, and one of the
many global scope IP addresses on any interface, get delivered to the
CPU, when the accelerator is L3 capable. If the IP address does not
match, it gets routed in hardware, if there is an appropriate router,
otherwise it get passed to the CPU, so the CPU can route it out an
interface which is not part of the switch.
> Look, this again boils down to what promisc mode means with regards to
> hardware offload. You want it to mean punt all traffic to the CPU? Fine.
> Does not seem like anyone will be switching sides anyway, so lets move
> forward. But the current approach is not good. Each driver needs to have
> this special case logic and the semantics of promisc mode change not
> only with regards to the value of the promisc counter, but also with
> regards to the interface's uppers. This is highly fragile and confusing.
Yes, i agree. We want one function, in the core, which handles all the
different uppers. Maybe 2, if we need to consider L2 and L3 switches
differently.
Andrew
^ permalink raw reply
* Re: [PATCH] netlabel: remove redundant assignment to pointer iter
From: David Miller @ 2019-09-01 18:45 UTC (permalink / raw)
To: colin.king
Cc: paul, netdev, linux-security-module, kernel-janitors,
linux-kernel
In-Reply-To: <20190901155205.16877-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Sun, 1 Sep 2019 16:52:05 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> Pointer iter is being initialized with a value that is never read and
> is being re-assigned a little later on. The assignment is redundant
> and hence can be removed.
>
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH net] isdn/capi: check message length in capi_write()
From: David Miller @ 2019-09-01 18:44 UTC (permalink / raw)
To: ebiggers; +Cc: netdev, isdn, syzkaller-bugs
In-Reply-To: <20190901143239.13828-1-ebiggers@kernel.org>
From: Eric Biggers <ebiggers@kernel.org>
Date: Sun, 1 Sep 2019 09:32:39 -0500
> From: Eric Biggers <ebiggers@google.com>
>
> syzbot reported:
>
> BUG: KMSAN: uninit-value in capi_write+0x791/0xa90 drivers/isdn/capi/capi.c:700
> CPU: 0 PID: 10025 Comm: syz-executor379 Not tainted 4.20.0-rc7+ #2
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x173/0x1d0 lib/dump_stack.c:113
> kmsan_report+0x12e/0x2a0 mm/kmsan/kmsan.c:613
> __msan_warning+0x82/0xf0 mm/kmsan/kmsan_instr.c:313
> capi_write+0x791/0xa90 drivers/isdn/capi/capi.c:700
> do_loop_readv_writev fs/read_write.c:703 [inline]
> do_iter_write+0x83e/0xd80 fs/read_write.c:961
> vfs_writev fs/read_write.c:1004 [inline]
> do_writev+0x397/0x840 fs/read_write.c:1039
> __do_sys_writev fs/read_write.c:1112 [inline]
> __se_sys_writev+0x9b/0xb0 fs/read_write.c:1109
> __x64_sys_writev+0x4a/0x70 fs/read_write.c:1109
> do_syscall_64+0xbc/0xf0 arch/x86/entry/common.c:291
> entry_SYSCALL_64_after_hwframe+0x63/0xe7
> [...]
>
> The problem is that capi_write() is reading past the end of the message.
> Fix it by checking the message's length in the needed places.
>
> Reported-and-tested-by: syzbot+0849c524d9c634f5ae66@syzkaller.appspotmail.com
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> drivers/isdn/capi/capi.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
> index 3c3ad42f22bf..a016d8c3c410 100644
> --- a/drivers/isdn/capi/capi.c
> +++ b/drivers/isdn/capi/capi.c
> @@ -688,6 +688,9 @@ capi_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos
> if (!cdev->ap.applid)
> return -ENODEV;
>
> + if (count < CAPIMSG_BASELEN)
> + return -EINVAL;
> +
> skb = alloc_skb(count, GFP_USER);
> if (!skb)
> return -ENOMEM;
This is fine.
> @@ -698,7 +701,8 @@ capi_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos
> }
> mlen = CAPIMSG_LEN(skb->data);
> if (CAPIMSG_CMD(skb->data) == CAPI_DATA_B3_REQ) {
> - if ((size_t)(mlen + CAPIMSG_DATALEN(skb->data)) != count) {
> + if (count < 18 ||
...
> + if (count < 12) {
These magic constants, on the other hand, are not.
^ permalink raw reply
* Re: [PATCH net-next] r8169: don't set bit RxVlan on RTL8125
From: David Miller @ 2019-09-01 18:37 UTC (permalink / raw)
To: hkallweit1; +Cc: nic_swsd, netdev
In-Reply-To: <84b91849-0ea6-5f76-150e-bee20a2a4d5c@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Sun, 1 Sep 2019 10:42:44 +0200
> RTL8125 uses a different register for VLAN offloading config,
> therefore don't set bit RxVlan.
>
> Fixes: f1bce4ad2f1c ("r8169: add support for RTL8125")
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH] netlabel: remove redundant assignment to pointer iter
From: Paul Moore @ 2019-09-01 18:23 UTC (permalink / raw)
To: Colin King
Cc: David S . Miller, netdev, linux-security-module, kernel-janitors,
linux-kernel
In-Reply-To: <20190901155205.16877-1-colin.king@canonical.com>
On Sun, Sep 1, 2019 at 11:52 AM Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Pointer iter is being initialized with a value that is never read and
> is being re-assigned a little later on. The assignment is redundant
> and hence can be removed.
>
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> net/netlabel/netlabel_kapi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Paul Moore <paul@paul-moore.com>
> diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
> index 2b0ef55cf89e..409a3ae47ce2 100644
> --- a/net/netlabel/netlabel_kapi.c
> +++ b/net/netlabel/netlabel_kapi.c
> @@ -607,7 +607,7 @@ static struct netlbl_lsm_catmap *_netlbl_catmap_getnode(
> */
> int netlbl_catmap_walk(struct netlbl_lsm_catmap *catmap, u32 offset)
> {
> - struct netlbl_lsm_catmap *iter = catmap;
> + struct netlbl_lsm_catmap *iter;
> u32 idx;
> u32 bit;
> NETLBL_CATMAP_MAPTYPE bitmap;
> --
> 2.20.1
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH] netlabel: remove redundant assignment to pointer iter
From: Paul Moore @ 2019-09-01 18:22 UTC (permalink / raw)
To: Christophe JAILLET
Cc: Colin King, David S . Miller, netdev, linux-security-module,
kernel-janitors, linux-kernel
In-Reply-To: <de0cd774-8fce-d69a-8ea4-3c7b2ee3a918@wanadoo.fr>
On Sun, Sep 1, 2019 at 1:16 PM Christophe JAILLET
<christophe.jaillet@wanadoo.fr> wrote:
> Le 01/09/2019 à 18:04, Paul Moore a écrit :
>
> On Sun, Sep 1, 2019 at 11:52 AM Colin King <colin.king@canonical.com> wrote:
>
> From: Colin Ian King <colin.king@canonical.com>
>
> Pointer iter is being initialized with a value that is never read and
> is being re-assigned a little later on. The assignment is redundant
> and hence can be removed.
>
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> net/netlabel/netlabel_kapi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> This patch doesn't seem correct to me, at least not in current form.
> At the top of _netlbl_catmap_getnode() is a check to see if iter is
> NULL (as well as a few other checks on iter after that); this patch
> would break that code.
>
> Perhaps we can get rid of the iter/catmap assignment when we define
> iter, but I don't think this patch is the right way to do it.
>
> diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
> index 2b0ef55cf89e..409a3ae47ce2 100644
> --- a/net/netlabel/netlabel_kapi.c
> +++ b/net/netlabel/netlabel_kapi.c
> @@ -607,7 +607,7 @@ static struct netlbl_lsm_catmap *_netlbl_catmap_getnode(
> */
> int netlbl_catmap_walk(struct netlbl_lsm_catmap *catmap, u32 offset)
> {
> - struct netlbl_lsm_catmap *iter = catmap;
> + struct netlbl_lsm_catmap *iter;
> u32 idx;
> u32 bit;
> NETLBL_CATMAP_MAPTYPE bitmap;
> --
> 2.20.1
>
> 'iter' is reassigned a value between the declaration and the NULL test, so removing the fist initialisation looks good to me.
This is what I get when I try to review patches quickly while doing
other things on the weekend <sigh> ... yes, you are correct, I was
looking at _netlbl_catmap_getnode() and not netlbl_catmap_walk(); my
apologies.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: WARNING in __mark_chain_precision (2)
From: syzbot @ 2019-09-01 18:23 UTC (permalink / raw)
To: allison, arvid.brodin, ast, bpf, clang-built-linux, daniel, davem,
gregkh, hawk, jakub.kicinski, jic23, john.fastabend, kafai,
knaack.h, kstewart, lars, linus.walleij, linux-iio, linux-kernel,
mchehab+samsung, netdev, nicolas.ferre, paulmck, pmeerw, rfontana,
songliubraving, syzkaller-bugs, tglx, torvalds, yhs
In-Reply-To: <000000000000b7a14105913fcca8@google.com>
syzbot has bisected this bug to:
commit e786741ff1b52769b044b7f4407f39cd13ee5d2d
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date: Thu Jul 11 22:36:02 2019 +0000
Merge tag 'staging-5.3-rc1' of
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=11958f12600000
start commit: 47ee6e86 selftests/bpf: remove wrong nhoff in flow dissect..
git tree: bpf-next
final crash: https://syzkaller.appspot.com/x/report.txt?x=13958f12600000
console output: https://syzkaller.appspot.com/x/log.txt?x=15958f12600000
kernel config: https://syzkaller.appspot.com/x/.config?x=d4cf1ffb87d590d7
dashboard link: https://syzkaller.appspot.com/bug?extid=c8d66267fd2b5955287e
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10d26ebc600000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=127805ca600000
Reported-by: syzbot+c8d66267fd2b5955287e@syzkaller.appspotmail.com
Fixes: e786741ff1b5 ("Merge tag 'staging-5.3-rc1' of
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
^ permalink raw reply
* Re: [PATCH V5 0/9] Fixes for vhost metadata acceleration
From: Michael S. Tsirkin @ 2019-09-01 18:02 UTC (permalink / raw)
To: Jason Wang; +Cc: kvm, virtualization, netdev, linux-kernel, linux-mm, jgg
In-Reply-To: <9325de4b-1d79-eb19-306e-e7a8fa8cc1a5@redhat.com>
On Tue, Aug 20, 2019 at 10:29:32AM +0800, Jason Wang wrote:
>
> On 2019/8/20 上午5:08, Michael S. Tsirkin wrote:
> > On Tue, Aug 13, 2019 at 04:12:49PM +0800, Jason Wang wrote:
> > > On 2019/8/12 下午5:49, Michael S. Tsirkin wrote:
> > > > On Mon, Aug 12, 2019 at 10:44:51AM +0800, Jason Wang wrote:
> > > > > On 2019/8/11 上午1:52, Michael S. Tsirkin wrote:
> > > > > > On Fri, Aug 09, 2019 at 01:48:42AM -0400, Jason Wang wrote:
> > > > > > > Hi all:
> > > > > > >
> > > > > > > This series try to fix several issues introduced by meta data
> > > > > > > accelreation series. Please review.
> > > > > > >
> > > > > > > Changes from V4:
> > > > > > > - switch to use spinlock synchronize MMU notifier with accessors
> > > > > > >
> > > > > > > Changes from V3:
> > > > > > > - remove the unnecessary patch
> > > > > > >
> > > > > > > Changes from V2:
> > > > > > > - use seqlck helper to synchronize MMU notifier with vhost worker
> > > > > > >
> > > > > > > Changes from V1:
> > > > > > > - try not use RCU to syncrhonize MMU notifier with vhost worker
> > > > > > > - set dirty pages after no readers
> > > > > > > - return -EAGAIN only when we find the range is overlapped with
> > > > > > > metadata
> > > > > > >
> > > > > > > Jason Wang (9):
> > > > > > > vhost: don't set uaddr for invalid address
> > > > > > > vhost: validate MMU notifier registration
> > > > > > > vhost: fix vhost map leak
> > > > > > > vhost: reset invalidate_count in vhost_set_vring_num_addr()
> > > > > > > vhost: mark dirty pages during map uninit
> > > > > > > vhost: don't do synchronize_rcu() in vhost_uninit_vq_maps()
> > > > > > > vhost: do not use RCU to synchronize MMU notifier with worker
> > > > > > > vhost: correctly set dirty pages in MMU notifiers callback
> > > > > > > vhost: do not return -EAGAIN for non blocking invalidation too early
> > > > > > >
> > > > > > > drivers/vhost/vhost.c | 202 +++++++++++++++++++++++++-----------------
> > > > > > > drivers/vhost/vhost.h | 6 +-
> > > > > > > 2 files changed, 122 insertions(+), 86 deletions(-)
> > > > > > This generally looks more solid.
> > > > > >
> > > > > > But this amounts to a significant overhaul of the code.
> > > > > >
> > > > > > At this point how about we revert 7f466032dc9e5a61217f22ea34b2df932786bbfc
> > > > > > for this release, and then re-apply a corrected version
> > > > > > for the next one?
> > > > > If possible, consider we've actually disabled the feature. How about just
> > > > > queued those patches for next release?
> > > > >
> > > > > Thanks
> > > > Sorry if I was unclear. My idea is that
> > > > 1. I revert the disabled code
> > > > 2. You send a patch readding it with all the fixes squashed
> > > > 3. Maybe optimizations on top right away?
> > > > 4. We queue *that* for next and see what happens.
> > > >
> > > > And the advantage over the patchy approach is that the current patches
> > > > are hard to review. E.g. it's not reasonable to ask RCU guys to review
> > > > the whole of vhost for RCU usage but it's much more reasonable to ask
> > > > about a specific patch.
> > >
> > > Ok. Then I agree to revert.
> > >
> > > Thanks
> > Great, so please send the following:
> > - revert
> > - squashed and fixed patch
>
>
> Just to confirm, do you want me to send a single series or two?
>
> Thanks
>
One is fine.
--
MST
^ permalink raw reply
* Re: [PATCH] net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)
From: Maciej Żenczykowski @ 2019-09-01 17:55 UTC (permalink / raw)
To: Maciej Żenczykowski, David S . Miller; +Cc: Linux NetDev, David Ahern
In-Reply-To: <20190901174759.257032-1-zenczykowski@gmail.com>
Some background:
This was found due to bad interactions with one of the few remaining
Android common kernel networking patches.
(The one that makes it possible for RA's to create routes in interface
specific tables)
The cleanup portion of it scours all tables and deletes all relevant
ADDRCONF routes, which in 5.2-rc1+ now includes ::1/128 and thus
terribly breaks things (in the Android Kernel Networking tests).
However, it *is* a userspace visible change in behaviour (since it's
visible via the above /proc file),
so one could argue for the above patch (or something similar).
The Android patch *could* also probably be adjusted to handle this
case (and thus prevent the breakage).
It's not immediately clear to me what is the better approach as I'm
not immediately certain what RTF_ADDRCONF truly means.
However the in kernel header file comment does explicitly mention this
being used to flag routes derived from RA's, and very clearly ::1/128
is not RA generated, so I *think* the correct fix is to return to the
old way the kernel used to do things and not flag with ADDRCONF...
Opinions?
^ permalink raw reply
* [PATCH] net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)
From: Maciej Żenczykowski @ 2019-09-01 17:47 UTC (permalink / raw)
To: Maciej Żenczykowski, David S . Miller; +Cc: netdev, David Ahern
From: Maciej Żenczykowski <maze@google.com>
There is a subtle change in behaviour introduced by:
commit c7a1ce397adacaf5d4bb2eab0a738b5f80dc3e43
'ipv6: Change addrconf_f6i_alloc to use ip6_route_info_create'
Before that patch /proc/net/ipv6_route includes:
00000000000000000000000000000001 80 00000000000000000000000000000000 00 00000000000000000000000000000000 00000000 00000003 00000000 80200001 lo
Afterwards /proc/net/ipv6_route includes:
00000000000000000000000000000001 80 00000000000000000000000000000000 00 00000000000000000000000000000000 00000000 00000002 00000000 80240001 lo
ie. the above commit causes the ::/128 local (automatic) route to be flagged with RTF_ADDRCONF (0x040000).
AFAICT, this is incorrect since these routes are *not* coming from RA's.
As such, this patch restores the old behaviour.
Fixes: c7a1ce397adacaf5d4bb2eab0a738b5f80dc3e43
Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Maciej Żenczykowski <maze@google.com>
---
net/ipv6/route.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 558c6c68855f..cee977e52d34 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -4365,13 +4365,14 @@ struct fib6_info *addrconf_f6i_alloc(struct net *net,
struct fib6_config cfg = {
.fc_table = l3mdev_fib_table(idev->dev) ? : RT6_TABLE_LOCAL,
.fc_ifindex = idev->dev->ifindex,
- .fc_flags = RTF_UP | RTF_ADDRCONF | RTF_NONEXTHOP,
+ .fc_flags = RTF_UP | RTF_NONEXTHOP,
.fc_dst = *addr,
.fc_dst_len = 128,
.fc_protocol = RTPROT_KERNEL,
.fc_nlinfo.nl_net = net,
.fc_ignore_dev_down = true,
};
+ struct fib6_info *f6i;
if (anycast) {
cfg.fc_type = RTN_ANYCAST;
@@ -4381,7 +4382,9 @@ struct fib6_info *addrconf_f6i_alloc(struct net *net,
cfg.fc_flags |= RTF_LOCAL;
}
- return ip6_route_info_create(&cfg, gfp_flags, NULL);
+ f6i = ip6_route_info_create(&cfg, gfp_flags, NULL);
+ f6i->dst_nocount = true;
+ return f6i;
}
/* remove deleted ip from prefsrc entries */
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related
* Re: [PATCH] netlabel: remove redundant assignment to pointer iter
From: Christophe JAILLET @ 2019-09-01 17:20 UTC (permalink / raw)
To: Paul Moore, Colin King
Cc: David S . Miller, netdev, linux-security-module, kernel-janitors,
linux-kernel
In-Reply-To: <CAHC9VhSVKEJ-EBAry5fVN3Ux22occGQ5jxbFBecMsR+q7+UT5A@mail.gmail.com>
Le 01/09/2019 à 18:04, Paul Moore a écrit :
> On Sun, Sep 1, 2019 at 11:52 AM Colin King <colin.king@canonical.com> wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> Pointer iter is being initialized with a value that is never read and
>> is being re-assigned a little later on. The assignment is redundant
>> and hence can be removed.
>>
>> Addresses-Coverity: ("Unused value")
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>> net/netlabel/netlabel_kapi.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
> This patch doesn't seem correct to me, at least not in current form.
> At the top of _netlbl_catmap_getnode() is a check to see if iter is
> NULL (as well as a few other checks on iter after that); this patch
> would break that code.
>
> Perhaps we can get rid of the iter/catmap assignment when we define
> iter, but I don't think this patch is the right way to do it.
>
>> diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
>> index 2b0ef55cf89e..409a3ae47ce2 100644
>> --- a/net/netlabel/netlabel_kapi.c
>> +++ b/net/netlabel/netlabel_kapi.c
>> @@ -607,7 +607,7 @@ static struct netlbl_lsm_catmap *_netlbl_catmap_getnode(
>> */
>> int netlbl_catmap_walk(struct netlbl_lsm_catmap *catmap, u32 offset)
>> {
>> - struct netlbl_lsm_catmap *iter = catmap;
>> + struct netlbl_lsm_catmap *iter;
>> u32 idx;
>> u32 bit;
>> NETLBL_CATMAP_MAPTYPE bitmap;
>> --
>> 2.20.1
>
Hi,
'iter' is reassigned a value between the declaration and the NULL test, so removing the first initialization looks good to me.
int netlbl_catmap_walk(struct netlbl_lsm_catmap *catmap, u32 offset)
{
|
struct netlbl_lsm_catmap *iter = catmap;
u32 idx;
u32 bit;
NETLBL_CATMAP_MAPTYPE bitmap;
iter = _netlbl_catmap_getnode(&catmap, offset, _CM_F_WALK, 0); <-- Here
if (iter == NULL)
return -ENOENT; This is dead code since commit d960a6184a92 ("netlabel: fix the catmap walking functions") where the call to _netlbl_catmap_getnode has been introduced.
Just my 2c,
CJ|
^ permalink raw reply
* Re: [PATCH] netlabel: remove redundant assignment to pointer iter
From: Paul Moore @ 2019-09-01 16:04 UTC (permalink / raw)
To: Colin King
Cc: David S . Miller, netdev, linux-security-module, kernel-janitors,
linux-kernel
In-Reply-To: <20190901155205.16877-1-colin.king@canonical.com>
On Sun, Sep 1, 2019 at 11:52 AM Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Pointer iter is being initialized with a value that is never read and
> is being re-assigned a little later on. The assignment is redundant
> and hence can be removed.
>
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> net/netlabel/netlabel_kapi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
This patch doesn't seem correct to me, at least not in current form.
At the top of _netlbl_catmap_getnode() is a check to see if iter is
NULL (as well as a few other checks on iter after that); this patch
would break that code.
Perhaps we can get rid of the iter/catmap assignment when we define
iter, but I don't think this patch is the right way to do it.
> diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
> index 2b0ef55cf89e..409a3ae47ce2 100644
> --- a/net/netlabel/netlabel_kapi.c
> +++ b/net/netlabel/netlabel_kapi.c
> @@ -607,7 +607,7 @@ static struct netlbl_lsm_catmap *_netlbl_catmap_getnode(
> */
> int netlbl_catmap_walk(struct netlbl_lsm_catmap *catmap, u32 offset)
> {
> - struct netlbl_lsm_catmap *iter = catmap;
> + struct netlbl_lsm_catmap *iter;
> u32 idx;
> u32 bit;
> NETLBL_CATMAP_MAPTYPE bitmap;
> --
> 2.20.1
--
paul moore
www.paul-moore.com
^ permalink raw reply
* [PATCH] netlabel: remove redundant assignment to pointer iter
From: Colin King @ 2019-09-01 15:52 UTC (permalink / raw)
To: Paul Moore, David S . Miller, netdev, linux-security-module
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Pointer iter is being initialized with a value that is never read and
is being re-assigned a little later on. The assignment is redundant
and hence can be removed.
Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
net/netlabel/netlabel_kapi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
index 2b0ef55cf89e..409a3ae47ce2 100644
--- a/net/netlabel/netlabel_kapi.c
+++ b/net/netlabel/netlabel_kapi.c
@@ -607,7 +607,7 @@ static struct netlbl_lsm_catmap *_netlbl_catmap_getnode(
*/
int netlbl_catmap_walk(struct netlbl_lsm_catmap *catmap, u32 offset)
{
- struct netlbl_lsm_catmap *iter = catmap;
+ struct netlbl_lsm_catmap *iter;
u32 idx;
u32 bit;
NETLBL_CATMAP_MAPTYPE bitmap;
--
2.20.1
^ permalink raw reply related
* [PATCH net] isdn/capi: check message length in capi_write()
From: Eric Biggers @ 2019-09-01 14:32 UTC (permalink / raw)
To: netdev, David S . Miller; +Cc: Karsten Keil, syzkaller-bugs
In-Reply-To: <0000000000000e35f00581a579cd@google.com>
From: Eric Biggers <ebiggers@google.com>
syzbot reported:
BUG: KMSAN: uninit-value in capi_write+0x791/0xa90 drivers/isdn/capi/capi.c:700
CPU: 0 PID: 10025 Comm: syz-executor379 Not tainted 4.20.0-rc7+ #2
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x173/0x1d0 lib/dump_stack.c:113
kmsan_report+0x12e/0x2a0 mm/kmsan/kmsan.c:613
__msan_warning+0x82/0xf0 mm/kmsan/kmsan_instr.c:313
capi_write+0x791/0xa90 drivers/isdn/capi/capi.c:700
do_loop_readv_writev fs/read_write.c:703 [inline]
do_iter_write+0x83e/0xd80 fs/read_write.c:961
vfs_writev fs/read_write.c:1004 [inline]
do_writev+0x397/0x840 fs/read_write.c:1039
__do_sys_writev fs/read_write.c:1112 [inline]
__se_sys_writev+0x9b/0xb0 fs/read_write.c:1109
__x64_sys_writev+0x4a/0x70 fs/read_write.c:1109
do_syscall_64+0xbc/0xf0 arch/x86/entry/common.c:291
entry_SYSCALL_64_after_hwframe+0x63/0xe7
[...]
The problem is that capi_write() is reading past the end of the message.
Fix it by checking the message's length in the needed places.
Reported-and-tested-by: syzbot+0849c524d9c634f5ae66@syzkaller.appspotmail.com
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
drivers/isdn/capi/capi.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 3c3ad42f22bf..a016d8c3c410 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -688,6 +688,9 @@ capi_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos
if (!cdev->ap.applid)
return -ENODEV;
+ if (count < CAPIMSG_BASELEN)
+ return -EINVAL;
+
skb = alloc_skb(count, GFP_USER);
if (!skb)
return -ENOMEM;
@@ -698,7 +701,8 @@ capi_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos
}
mlen = CAPIMSG_LEN(skb->data);
if (CAPIMSG_CMD(skb->data) == CAPI_DATA_B3_REQ) {
- if ((size_t)(mlen + CAPIMSG_DATALEN(skb->data)) != count) {
+ if (count < 18 ||
+ (size_t)(mlen + CAPIMSG_DATALEN(skb->data)) != count) {
kfree_skb(skb);
return -EINVAL;
}
@@ -711,6 +715,10 @@ capi_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos
CAPIMSG_SETAPPID(skb->data, cdev->ap.applid);
if (CAPIMSG_CMD(skb->data) == CAPI_DISCONNECT_B3_RESP) {
+ if (count < 12) {
+ kfree_skb(skb);
+ return -EINVAL;
+ }
mutex_lock(&cdev->lock);
capincci_free(cdev, CAPIMSG_NCCI(skb->data));
mutex_unlock(&cdev->lock);
--
2.23.0
^ permalink raw reply related
* Re: [PATCH] Fix a double free bug in rsi_91x_deinit
From: Guenter Roeck @ 2019-09-01 14:08 UTC (permalink / raw)
To: Kalle Valo
Cc: Hui Peng, security, Mathias Payer, David S. Miller,
linux-wireless, netdev, linux-kernel
In-Reply-To: <87k1asqw87.fsf@kamboji.qca.qualcomm.com>
On 9/1/19 1:03 AM, Kalle Valo wrote:
> Guenter Roeck <linux@roeck-us.net> writes:
>
>> On Mon, Aug 19, 2019 at 06:02:29PM -0400, Hui Peng wrote:
>>> `dev` (struct rsi_91x_usbdev *) field of adapter
>>> (struct rsi_91x_usbdev *) is allocated and initialized in
>>> `rsi_init_usb_interface`. If any error is detected in information
>>> read from the device side, `rsi_init_usb_interface` will be
>>> freed. However, in the higher level error handling code in
>>> `rsi_probe`, if error is detected, `rsi_91x_deinit` is called
>>> again, in which `dev` will be freed again, resulting double free.
>>>
>>> This patch fixes the double free by removing the free operation on
>>> `dev` in `rsi_init_usb_interface`, because `rsi_91x_deinit` is also
>>> used in `rsi_disconnect`, in that code path, the `dev` field is not
>>> (and thus needs to be) freed.
>>>
>>> This bug was found in v4.19, but is also present in the latest version
>>> of kernel.
>>>
>>> Reported-by: Hui Peng <benquike@gmail.com>
>>> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
>>> Signed-off-by: Hui Peng <benquike@gmail.com>
>>
>> FWIW:
>>
>> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>>
>> This patch is listed as fix for CVE-2019-15504, which has a CVSS 2.0 score
>> of 10.0 (high) and CVSS 3.0 score of 9.8 (critical).
>
> A double free in error path is considered as a critical CVE issue? I'm
> very curious, why is that?
>
You'd have to ask the people assigning CVSS scores. However, if the memory
was reallocated, that reallocated memory (which is still in use) is freed.
Then all kinds of bad things can happen.
Guenter
>> Are there any plans to apply this patch to the upstream kernel anytime
>> soon ?
>
> I was on vacation last week and hence I have not been able to apply any
> wireless patches. I should be able to catch up next week.
>
^ permalink raw reply
* [PATCH V2 3/4] firmware: xilinx: Add ZynqMP aes API for AES functionality
From: Kalyani Akula @ 2019-09-01 13:54 UTC (permalink / raw)
To: herbert, kstewart, gregkh, tglx, pombredanne, linux-crypto,
linux-kernel, netdev
Cc: Kalyani Akula, Kalyani Akula
In-Reply-To: <1567346098-27927-1-git-send-email-kalyani.akula@xilinx.com>
Add ZynqMP firmware AES API to perform encryption/decryption
of given data.
Signed-off-by: Kalyani Akula <kalyani.akula@xilinx.com>
---
drivers/firmware/xilinx/zynqmp.c | 23 +++++++++++++++++++++++
include/linux/firmware/xlnx-zynqmp.h | 2 ++
2 files changed, 25 insertions(+)
diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index fd3d837..74f3354 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -663,6 +663,28 @@ static int zynqmp_pm_set_requirement(const u32 node, const u32 capabilities,
return zynqmp_pm_invoke_fn(PM_SET_REQUIREMENT, node, capabilities,
qos, ack, NULL);
}
+/**
+ * zynqmp_pm_aes - Access AES hardware to encrypt/decrypt the data using
+ * AES-GCM core.
+ * @address: Address of the AesParams structure.
+ * @out: Returned output value
+ *
+ * Return: Returns status, either success or error code.
+ */
+static int zynqmp_pm_aes_engine(const u64 address, u32 *out)
+{
+ u32 ret_payload[PAYLOAD_ARG_CNT];
+ int ret;
+
+ if (!out)
+ return -EINVAL;
+
+ ret = zynqmp_pm_invoke_fn(PM_SECURE_AES, upper_32_bits(address),
+ lower_32_bits(address),
+ 0, 0, ret_payload);
+ *out = ret_payload[1];
+ return ret;
+}
static const struct zynqmp_eemi_ops eemi_ops = {
.get_api_version = zynqmp_pm_get_api_version,
@@ -687,6 +709,7 @@ static int zynqmp_pm_set_requirement(const u32 node, const u32 capabilities,
.set_requirement = zynqmp_pm_set_requirement,
.fpga_load = zynqmp_pm_fpga_load,
.fpga_get_status = zynqmp_pm_fpga_get_status,
+ .aes = zynqmp_pm_aes_engine,
};
/**
diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index 778abbb..508edd7 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -77,6 +77,7 @@ enum pm_api_id {
PM_CLOCK_GETRATE,
PM_CLOCK_SETPARENT,
PM_CLOCK_GETPARENT,
+ PM_SECURE_AES = 47,
};
/* PMU-FW return status codes */
@@ -294,6 +295,7 @@ struct zynqmp_eemi_ops {
const u32 capabilities,
const u32 qos,
const enum zynqmp_pm_request_ack ack);
+ int (*aes)(const u64 address, u32 *out);
};
int zynqmp_pm_invoke_fn(u32 pm_api_id, u32 arg0, u32 arg1,
--
1.8.3.1
^ permalink raw reply related
* [PATCH V2 1/4] dt-bindings: crypto: Add bindings for ZynqMP AES driver
From: Kalyani Akula @ 2019-09-01 13:54 UTC (permalink / raw)
To: herbert, kstewart, gregkh, tglx, pombredanne, linux-crypto,
linux-kernel, netdev
Cc: Kalyani Akula, Kalyani Akula
In-Reply-To: <1567346098-27927-1-git-send-email-kalyani.akula@xilinx.com>
Add documentation to describe Xilinx ZynqMP AES driver
bindings.
Signed-off-by: Kalyani Akula <kalyani.akula@xilinx.com>
---
Documentation/devicetree/bindings/crypto/xlnx,zynqmp-aes.txt | 12 ++++++++++++
1 file changed, 12 insertions(+)
create mode 100644 Documentation/devicetree/bindings/crypto/xlnx,zynqmp-aes.txt
diff --git a/Documentation/devicetree/bindings/crypto/xlnx,zynqmp-aes.txt b/Documentation/devicetree/bindings/crypto/xlnx,zynqmp-aes.txt
new file mode 100644
index 0000000..226bfb9
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/xlnx,zynqmp-aes.txt
@@ -0,0 +1,12 @@
+Xilinx ZynqMP AES hw acceleration support
+
+The ZynqMP PS-AES hw accelerator is used to encrypt/decrypt
+the given user data.
+
+Required properties:
+- compatible: should contain "xlnx,zynqmp-aes"
+
+Example:
+ zynqmp_aes {
+ compatible = "xlnx,zynqmp-aes";
+ };
--
1.8.3.1
^ permalink raw reply related
* [PATCH V2 2/4] ARM64: zynqmp: Add Xilinix AES node.
From: Kalyani Akula @ 2019-09-01 13:54 UTC (permalink / raw)
To: herbert, kstewart, gregkh, tglx, pombredanne, linux-crypto,
linux-kernel, netdev
Cc: Kalyani Akula, Kalyani Akula
In-Reply-To: <1567346098-27927-1-git-send-email-kalyani.akula@xilinx.com>
This patch adds a AES DT node for Xilinx ZynqMP SoC.
Signed-off-by: Kalyani Akula <kalyani.akula@xilinx.com>
---
arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
index 9aa6734..b41febc 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
+++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
@@ -124,6 +124,10 @@
<1 10 0xf08>;
};
+ xlnx_aes: zynqmp_aes {
+ compatible = "xlnx,zynqmp-aes";
+ };
+
amba_apu: amba-apu@0 {
compatible = "simple-bus";
#address-cells = <2>;
--
1.8.3.1
^ permalink raw reply related
* [PATCH V2 4/4] crypto: Add Xilinx AES driver
From: Kalyani Akula @ 2019-09-01 13:54 UTC (permalink / raw)
To: herbert, kstewart, gregkh, tglx, pombredanne, linux-crypto,
linux-kernel, netdev
Cc: Kalyani Akula, Kalyani Akula
In-Reply-To: <1567346098-27927-1-git-send-email-kalyani.akula@xilinx.com>
This patch adds AES driver support for the Xilinx
ZynqMP SoC.
Signed-off-by: Kalyani Akula <kalyani.akula@xilinx.com>
---
drivers/crypto/Kconfig | 11 ++
drivers/crypto/Makefile | 1 +
drivers/crypto/zynqmp-aes-gcm.c | 297 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 309 insertions(+)
create mode 100644 drivers/crypto/zynqmp-aes-gcm.c
diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 603413f..a0d058a 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -677,6 +677,17 @@ config CRYPTO_DEV_ROCKCHIP
This driver interfaces with the hardware crypto accelerator.
Supporting cbc/ecb chainmode, and aes/des/des3_ede cipher mode.
+config CRYPTO_DEV_ZYNQMP_AES
+ tristate "Support for Xilinx ZynqMP AES hw accelerator"
+ depends on ARCH_ZYNQMP || COMPILE_TEST
+ select CRYPTO_AES
+ select CRYPTO_SKCIPHER
+ help
+ Xilinx ZynqMP has AES-GCM engine used for symmetric key
+ encryption and decryption. This driver interfaces with AES hw
+ accelerator. Select this if you want to use the ZynqMP module
+ for AES algorithms.
+
config CRYPTO_DEV_MEDIATEK
tristate "MediaTek's EIP97 Cryptographic Engine driver"
depends on (ARM && ARCH_MEDIATEK) || COMPILE_TEST
diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
index afc4753..c99663a 100644
--- a/drivers/crypto/Makefile
+++ b/drivers/crypto/Makefile
@@ -48,3 +48,4 @@ obj-$(CONFIG_CRYPTO_DEV_BCM_SPU) += bcm/
obj-$(CONFIG_CRYPTO_DEV_SAFEXCEL) += inside-secure/
obj-$(CONFIG_CRYPTO_DEV_ARTPEC6) += axis/
obj-y += hisilicon/
+obj-$(CONFIG_CRYPTO_DEV_ZYNQMP_AES) += zynqmp-aes-gcm.o
diff --git a/drivers/crypto/zynqmp-aes-gcm.c b/drivers/crypto/zynqmp-aes-gcm.c
new file mode 100644
index 0000000..d65f038
--- /dev/null
+++ b/drivers/crypto/zynqmp-aes-gcm.c
@@ -0,0 +1,297 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx ZynqMP AES Driver.
+ * Copyright (c) 2019 Xilinx Inc.
+ */
+
+#include <crypto/aes.h>
+#include <crypto/scatterwalk.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/scatterlist.h>
+#include <linux/firmware/xlnx-zynqmp.h>
+
+#define ZYNQMP_AES_IV_SIZE 12
+#define ZYNQMP_AES_GCM_SIZE 16
+#define ZYNQMP_AES_KEY_SIZE 32
+
+#define ZYNQMP_AES_DECRYPT 0
+#define ZYNQMP_AES_ENCRYPT 1
+
+#define ZYNQMP_AES_KUP_KEY 0
+#define ZYNQMP_AES_DEVICE_KEY 1
+#define ZYNQMP_AES_PUF_KEY 2
+
+#define ZYNQMP_AES_GCM_TAG_MISMATCH_ERR 0x01
+#define ZYNQMP_AES_SIZE_ERR 0x06
+#define ZYNQMP_AES_WRONG_KEY_SRC_ERR 0x13
+#define ZYNQMP_AES_PUF_NOT_PROGRAMMED 0xE300
+
+#define ZYNQMP_AES_BLOCKSIZE 0x04
+
+static const struct zynqmp_eemi_ops *eemi_ops;
+struct zynqmp_aes_dev *aes_dd;
+
+struct zynqmp_aes_dev {
+ struct device *dev;
+};
+
+struct zynqmp_aes_op {
+ struct zynqmp_aes_dev *dd;
+ void *src;
+ void *dst;
+ int len;
+ u8 key[ZYNQMP_AES_KEY_SIZE];
+ u8 *iv;
+ u32 keylen;
+ u32 keytype;
+};
+
+struct zynqmp_aes_data {
+ u64 src;
+ u64 iv;
+ u64 key;
+ u64 dst;
+ u64 size;
+ u64 optype;
+ u64 keysrc;
+};
+
+static int zynqmp_setkey_blk(struct crypto_tfm *tfm, const u8 *key,
+ unsigned int len)
+{
+ struct zynqmp_aes_op *op = crypto_tfm_ctx(tfm);
+
+ if (((len != 1) && (len != ZYNQMP_AES_KEY_SIZE)) || (!key))
+ return -EINVAL;
+
+ if (len == 1) {
+ op->keytype = *key;
+
+ if ((op->keytype < ZYNQMP_AES_KUP_KEY) ||
+ (op->keytype > ZYNQMP_AES_PUF_KEY))
+ return -EINVAL;
+
+ } else if (len == ZYNQMP_AES_KEY_SIZE) {
+ op->keytype = ZYNQMP_AES_KUP_KEY;
+ op->keylen = len;
+ memcpy(op->key, key, len);
+ }
+
+ return 0;
+}
+
+static int zynqmp_aes_xcrypt(struct blkcipher_desc *desc,
+ struct scatterlist *dst,
+ struct scatterlist *src,
+ unsigned int nbytes,
+ unsigned int flags)
+{
+ struct zynqmp_aes_op *op = crypto_blkcipher_ctx(desc->tfm);
+ struct zynqmp_aes_dev *dd = aes_dd;
+ int err, ret, copy_bytes, src_data = 0, dst_data = 0;
+ dma_addr_t dma_addr, dma_addr_buf;
+ struct zynqmp_aes_data *abuf;
+ struct blkcipher_walk walk;
+ unsigned int data_size;
+ size_t dma_size;
+ char *kbuf;
+
+ if (!eemi_ops->aes)
+ return -ENOTSUPP;
+
+ if (op->keytype == ZYNQMP_AES_KUP_KEY)
+ dma_size = nbytes + ZYNQMP_AES_KEY_SIZE
+ + ZYNQMP_AES_IV_SIZE;
+ else
+ dma_size = nbytes + ZYNQMP_AES_IV_SIZE;
+
+ kbuf = dma_alloc_coherent(dd->dev, dma_size, &dma_addr, GFP_KERNEL);
+ if (!kbuf)
+ return -ENOMEM;
+
+ abuf = dma_alloc_coherent(dd->dev, sizeof(struct zynqmp_aes_data),
+ &dma_addr_buf, GFP_KERNEL);
+ if (!abuf) {
+ dma_free_coherent(dd->dev, dma_size, kbuf, dma_addr);
+ return -ENOMEM;
+ }
+
+ data_size = nbytes;
+ blkcipher_walk_init(&walk, dst, src, data_size);
+ err = blkcipher_walk_virt(desc, &walk);
+ op->iv = walk.iv;
+
+ while ((nbytes = walk.nbytes)) {
+ op->src = walk.src.virt.addr;
+ memcpy(kbuf + src_data, op->src, nbytes);
+ src_data = src_data + nbytes;
+ nbytes &= (ZYNQMP_AES_BLOCKSIZE - 1);
+ err = blkcipher_walk_done(desc, &walk, nbytes);
+ }
+ memcpy(kbuf + data_size, op->iv, ZYNQMP_AES_IV_SIZE);
+ abuf->src = dma_addr;
+ abuf->dst = dma_addr;
+ abuf->iv = abuf->src + data_size;
+ abuf->size = data_size - ZYNQMP_AES_GCM_SIZE;
+ abuf->optype = flags;
+ abuf->keysrc = op->keytype;
+
+ if (op->keytype == ZYNQMP_AES_KUP_KEY) {
+ memcpy(kbuf + data_size + ZYNQMP_AES_IV_SIZE,
+ op->key, ZYNQMP_AES_KEY_SIZE);
+
+ abuf->key = abuf->src + data_size + ZYNQMP_AES_IV_SIZE;
+ } else {
+ abuf->key = 0;
+ }
+ eemi_ops->aes(dma_addr_buf, &ret);
+
+ if (ret != 0) {
+ switch (ret) {
+ case ZYNQMP_AES_GCM_TAG_MISMATCH_ERR:
+ dev_err(dd->dev, "ERROR: Gcm Tag mismatch\n\r");
+ break;
+ case ZYNQMP_AES_SIZE_ERR:
+ dev_err(dd->dev, "ERROR : Non word aligned data\n\r");
+ break;
+ case ZYNQMP_AES_WRONG_KEY_SRC_ERR:
+ dev_err(dd->dev, "ERROR: Wrong KeySrc, enable secure mode\n\r");
+ break;
+ case ZYNQMP_AES_PUF_NOT_PROGRAMMED:
+ dev_err(dd->dev, "ERROR: PUF is not registered\r\n");
+ break;
+ default:
+ dev_err(dd->dev, "ERROR: Invalid");
+ break;
+ }
+ goto END;
+ }
+ if (flags)
+ copy_bytes = data_size;
+ else
+ copy_bytes = data_size - ZYNQMP_AES_GCM_SIZE;
+
+ blkcipher_walk_init(&walk, dst, src, copy_bytes);
+ err = blkcipher_walk_virt(desc, &walk);
+
+ while ((nbytes = walk.nbytes)) {
+ memcpy(walk.dst.virt.addr, kbuf + dst_data, nbytes);
+ dst_data = dst_data + nbytes;
+ nbytes &= (ZYNQMP_AES_BLOCKSIZE - 1);
+ err = blkcipher_walk_done(desc, &walk, nbytes);
+ }
+END:
+ memset(kbuf, 0, dma_size);
+ memset(abuf, 0, sizeof(struct zynqmp_aes_data));
+ dma_free_coherent(dd->dev, dma_size, kbuf, dma_addr);
+ dma_free_coherent(dd->dev, sizeof(struct zynqmp_aes_data),
+ abuf, dma_addr_buf);
+ return err;
+}
+
+static int zynqmp_aes_decrypt(struct blkcipher_desc *desc,
+ struct scatterlist *dst,
+ struct scatterlist *src,
+ unsigned int nbytes)
+{
+ return zynqmp_aes_xcrypt(desc, dst, src, nbytes, ZYNQMP_AES_DECRYPT);
+}
+
+static int zynqmp_aes_encrypt(struct blkcipher_desc *desc,
+ struct scatterlist *dst,
+ struct scatterlist *src,
+ unsigned int nbytes)
+{
+ return zynqmp_aes_xcrypt(desc, dst, src, nbytes, ZYNQMP_AES_ENCRYPT);
+}
+
+static struct crypto_alg zynqmp_alg = {
+ .cra_name = "xilinx-zynqmp-aes",
+ .cra_driver_name = "zynqmp-aes-gcm",
+ .cra_priority = 400,
+ .cra_flags = CRYPTO_ALG_TYPE_BLKCIPHER |
+ CRYPTO_ALG_KERN_DRIVER_ONLY,
+ .cra_blocksize = ZYNQMP_AES_BLOCKSIZE,
+ .cra_ctxsize = sizeof(struct zynqmp_aes_op),
+ .cra_alignmask = 15,
+ .cra_type = &crypto_blkcipher_type,
+ .cra_module = THIS_MODULE,
+ .cra_u = {
+ .blkcipher = {
+ .min_keysize = 0,
+ .max_keysize = ZYNQMP_AES_KEY_SIZE,
+ .setkey = zynqmp_setkey_blk,
+ .encrypt = zynqmp_aes_encrypt,
+ .decrypt = zynqmp_aes_decrypt,
+ .ivsize = ZYNQMP_AES_IV_SIZE,
+ }
+ }
+};
+
+static const struct of_device_id zynqmp_aes_dt_ids[] = {
+ { .compatible = "xlnx,zynqmp-aes" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, zynqmp_aes_dt_ids);
+
+static int zynqmp_aes_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ int ret;
+
+ eemi_ops = zynqmp_pm_get_eemi_ops();
+
+ if (IS_ERR(eemi_ops))
+ return PTR_ERR(eemi_ops);
+
+ aes_dd = devm_kzalloc(dev, sizeof(*aes_dd), GFP_KERNEL);
+ if (!aes_dd)
+ return -ENOMEM;
+
+ aes_dd->dev = dev;
+ platform_set_drvdata(pdev, aes_dd);
+
+ ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(44));
+ if (ret < 0) {
+ dev_err(dev, "no usable DMA configuration");
+ return ret;
+ }
+
+ ret = crypto_register_alg(&zynqmp_alg);
+ if (ret)
+ goto err_algs;
+
+ dev_info(dev, "AES Successfully Registered\n\r");
+ return 0;
+
+err_algs:
+ return ret;
+}
+
+static int zynqmp_aes_remove(struct platform_device *pdev)
+{
+ aes_dd = platform_get_drvdata(pdev);
+ if (!aes_dd)
+ return -ENODEV;
+ crypto_unregister_alg(&zynqmp_alg);
+
+ return 0;
+}
+
+static struct platform_driver xilinx_aes_driver = {
+ .probe = zynqmp_aes_probe,
+ .remove = zynqmp_aes_remove,
+ .driver = {
+ .name = "zynqmp_aes",
+ .of_match_table = of_match_ptr(zynqmp_aes_dt_ids),
+ },
+};
+
+module_platform_driver(xilinx_aes_driver);
+
+MODULE_DESCRIPTION("Xilinx ZynqMP AES hw acceleration support.");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Nava kishore Manne <nava.manne@xilinx.com>");
+MODULE_AUTHOR("Kalyani Akula <kalyani.akula@xilinx.com>");
--
1.8.3.1
^ permalink raw reply related
* [PATCH V2 0/4] Add Xilinx's ZynqMP AES driver support
From: Kalyani Akula @ 2019-09-01 13:54 UTC (permalink / raw)
To: herbert, kstewart, gregkh, tglx, pombredanne, linux-crypto,
linux-kernel, netdev
Cc: Kalyani Akula, Kalyani Akula
This patch set adds support for
- dt-binding docs for Xilinx ZynqMP AES driver
- Adds device tree node for ZynqMP AES driver
- Adds communication layer support for aes in zynqmp.c
- Adds Xilinx ZynqMP driver for AES Algorithm
V2 Changes :
- Converted RFC PATCH to PATCH
- Removed ALG_SET_KEY_TYPE that was added to support keytype
attribute. Taken using setkey interface.
- Removed deprecated BLKCIPHER in Kconfig
- Erased Key/IV from the buffer.
- Renamed zynqmp-aes driver to zynqmp-aes-gcm.
- Addressed few other review comments
Kalyani Akula (4):
dt-bindings: crypto: Add bindings for ZynqMP AES driver
ARM64: zynqmp: Add Xilinix AES node.
firmware: xilinx: Add ZynqMP aes API for AES functionality
crypto: Add Xilinx AES driver
.../devicetree/bindings/crypto/xlnx,zynqmp-aes.txt | 12 +
arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 4 +
drivers/crypto/Kconfig | 11 +
drivers/crypto/Makefile | 1 +
drivers/crypto/zynqmp-aes-gcm.c | 297 +++++++++++++++++++++
drivers/firmware/xilinx/zynqmp.c | 23 ++
include/linux/firmware/xlnx-zynqmp.h | 2 +
7 files changed, 350 insertions(+)
create mode 100644 Documentation/devicetree/bindings/crypto/xlnx,zynqmp-aes.txt
create mode 100644 drivers/crypto/zynqmp-aes-gcm.c
--
1.8.3.1
^ permalink raw reply
* Re: [PATCH v4 1/5] vsock/virtio: limit the memory used per-socket
From: Michael S. Tsirkin @ 2019-09-01 10:17 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, linux-kernel, Stefan Hajnoczi, David S. Miller,
virtualization, Jason Wang, kvm
In-Reply-To: <20190901025815-mutt-send-email-mst@kernel.org>
On Sun, Sep 01, 2019 at 04:26:19AM -0400, Michael S. Tsirkin wrote:
> On Thu, Aug 01, 2019 at 03:36:16PM +0200, Stefano Garzarella wrote:
> > On Thu, Aug 01, 2019 at 09:21:15AM -0400, Michael S. Tsirkin wrote:
> > > On Thu, Aug 01, 2019 at 12:47:54PM +0200, Stefano Garzarella wrote:
> > > > On Tue, Jul 30, 2019 at 04:42:25PM -0400, Michael S. Tsirkin wrote:
> > > > > On Tue, Jul 30, 2019 at 11:35:39AM +0200, Stefano Garzarella wrote:
> > > >
> > > > (...)
> > > >
> > > > > >
> > > > > > The problem here is the compatibility. Before this series virtio-vsock
> > > > > > and vhost-vsock modules had the RX buffer size hard-coded
> > > > > > (VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE = 4K). So, if we send a buffer smaller
> > > > > > of 4K, there might be issues.
> > > > >
> > > > > Shouldn't be if they are following the spec. If not let's fix
> > > > > the broken parts.
> > > > >
> > > > > >
> > > > > > Maybe it is the time to add add 'features' to virtio-vsock device.
> > > > > >
> > > > > > Thanks,
> > > > > > Stefano
> > > > >
> > > > > Why would a remote care about buffer sizes?
> > > > >
> > > > > Let's first see what the issues are. If they exist
> > > > > we can either fix the bugs, or code the bug as a feature in spec.
> > > > >
> > > >
> > > > The vhost_transport '.stream_enqueue' callback
> > > > [virtio_transport_stream_enqueue()] calls the virtio_transport_send_pkt_info(),
> > > > passing the user message. This function allocates a new packet, copying
> > > > the user message, but (before this series) it limits the packet size to
> > > > the VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE (4K):
> > > >
> > > > static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> > > > struct virtio_vsock_pkt_info *info)
> > > > {
> > > > ...
> > > > /* we can send less than pkt_len bytes */
> > > > if (pkt_len > VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE)
> > > > pkt_len = VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE;
> > > >
> > > > /* virtio_transport_get_credit might return less than pkt_len credit */
> > > > pkt_len = virtio_transport_get_credit(vvs, pkt_len);
> > > >
> > > > /* Do not send zero length OP_RW pkt */
> > > > if (pkt_len == 0 && info->op == VIRTIO_VSOCK_OP_RW)
> > > > return pkt_len;
> > > > ...
> > > > }
> > > >
> > > > then it queues the packet for the TX worker calling .send_pkt()
> > > > [vhost_transport_send_pkt() in the vhost_transport case]
> > > >
> > > > The main function executed by the TX worker is
> > > > vhost_transport_do_send_pkt() that picks up a buffer from the virtqueue
> > > > and it tries to copy the packet (up to 4K) on it. If the buffer
> > > > allocated from the guest will be smaller then 4K, I think here it will
> > > > be discarded with an error:
> > > >
> >
> > I'm adding more lines to explain better.
> >
> > > > static void
> > > > vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
> > > > struct vhost_virtqueue *vq)
> > > > {
> > ...
> >
> > head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
> > &out, &in, NULL, NULL);
> >
> > ...
> >
> > len = iov_length(&vq->iov[out], in);
> > iov_iter_init(&iov_iter, READ, &vq->iov[out], in, len);
> >
> > nbytes = copy_to_iter(&pkt->hdr, sizeof(pkt->hdr), &iov_iter);
> > if (nbytes != sizeof(pkt->hdr)) {
> > virtio_transport_free_pkt(pkt);
> > vq_err(vq, "Faulted on copying pkt hdr\n");
> > break;
> > }
> >
> > > > ...
> > > > nbytes = copy_to_iter(pkt->buf, pkt->len, &iov_iter);
> > >
> > > isn't pck len the actual length though?
> > >
> >
> > It is the length of the packet that we are copying in the guest RX
> > buffers pointed by the iov_iter. The guest allocates an iovec with 2
> > buffers, one for the header and one for the payload (4KB).
>
> BTW at the moment that forces another kmalloc within virtio core. Maybe
> vsock needs a flag to skip allocation in this case. Worth benchmarking.
> See virtqueue_use_indirect which just does total_sg > 1.
>
> >
> > > > if (nbytes != pkt->len) {
> > > > virtio_transport_free_pkt(pkt);
> > > > vq_err(vq, "Faulted on copying pkt buf\n");
> > > > break;
> > > > }
> > > > ...
> > > > }
> > > >
> > > >
> > > > This series changes this behavior since now we will split the packet in
> > > > vhost_transport_do_send_pkt() depending on the buffer found in the
> > > > virtqueue.
> > > >
> > > > We didn't change the buffer size in this series, so we still backward
> > > > compatible, but if we will use buffers smaller than 4K, we should
> > > > encounter the error described above.
>
> So that's an implementation bug then? It made an assumption
> of a 4K sized buffer? Or even PAGE_SIZE sized buffer?
Assuming we miss nothing and buffers < 4K are broken,
I think we need to add this to the spec, possibly with
a feature bit to relax the requirement that all buffers
are at least 4k in size.
>
> > > >
> > > > How do you suggest we proceed if we want to change the buffer size?
> > > > Maybe adding a feature to "support any buffer size"?
> > > >
> > > > Thanks,
> > > > Stefano
> > >
> > >
> >
> > --
^ permalink raw reply
* [PATCH net-next] r8169: don't set bit RxVlan on RTL8125
From: Heiner Kallweit @ 2019-09-01 8:42 UTC (permalink / raw)
To: Realtek linux nic maintainers, David Miller; +Cc: netdev@vger.kernel.org
RTL8125 uses a different register for VLAN offloading config,
therefore don't set bit RxVlan.
Fixes: f1bce4ad2f1c ("r8169: add support for RTL8125")
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169_main.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 7d1094ffd..74f81fe03 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -7163,8 +7163,10 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
NETIF_F_HIGHDMA;
dev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
- tp->cp_cmd |= RxChkSum | RxVlan;
-
+ tp->cp_cmd |= RxChkSum;
+ /* RTL8125 uses register RxConfig for VLAN offloading config */
+ if (!rtl_is_8125(tp))
+ tp->cp_cmd |= RxVlan;
/*
* Pretend we are using VLANs; This bypasses a nasty bug where
* Interrupts stop flowing on high load on 8110SCd controllers.
--
2.23.0
^ permalink raw reply related
* Re: [PATCH v4 1/5] vsock/virtio: limit the memory used per-socket
From: Michael S. Tsirkin @ 2019-09-01 8:26 UTC (permalink / raw)
To: Stefano Garzarella
Cc: netdev, linux-kernel, Stefan Hajnoczi, David S. Miller,
virtualization, Jason Wang, kvm
In-Reply-To: <20190801133616.sik5drn6ecesukbb@steredhat>
On Thu, Aug 01, 2019 at 03:36:16PM +0200, Stefano Garzarella wrote:
> On Thu, Aug 01, 2019 at 09:21:15AM -0400, Michael S. Tsirkin wrote:
> > On Thu, Aug 01, 2019 at 12:47:54PM +0200, Stefano Garzarella wrote:
> > > On Tue, Jul 30, 2019 at 04:42:25PM -0400, Michael S. Tsirkin wrote:
> > > > On Tue, Jul 30, 2019 at 11:35:39AM +0200, Stefano Garzarella wrote:
> > >
> > > (...)
> > >
> > > > >
> > > > > The problem here is the compatibility. Before this series virtio-vsock
> > > > > and vhost-vsock modules had the RX buffer size hard-coded
> > > > > (VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE = 4K). So, if we send a buffer smaller
> > > > > of 4K, there might be issues.
> > > >
> > > > Shouldn't be if they are following the spec. If not let's fix
> > > > the broken parts.
> > > >
> > > > >
> > > > > Maybe it is the time to add add 'features' to virtio-vsock device.
> > > > >
> > > > > Thanks,
> > > > > Stefano
> > > >
> > > > Why would a remote care about buffer sizes?
> > > >
> > > > Let's first see what the issues are. If they exist
> > > > we can either fix the bugs, or code the bug as a feature in spec.
> > > >
> > >
> > > The vhost_transport '.stream_enqueue' callback
> > > [virtio_transport_stream_enqueue()] calls the virtio_transport_send_pkt_info(),
> > > passing the user message. This function allocates a new packet, copying
> > > the user message, but (before this series) it limits the packet size to
> > > the VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE (4K):
> > >
> > > static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
> > > struct virtio_vsock_pkt_info *info)
> > > {
> > > ...
> > > /* we can send less than pkt_len bytes */
> > > if (pkt_len > VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE)
> > > pkt_len = VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE;
> > >
> > > /* virtio_transport_get_credit might return less than pkt_len credit */
> > > pkt_len = virtio_transport_get_credit(vvs, pkt_len);
> > >
> > > /* Do not send zero length OP_RW pkt */
> > > if (pkt_len == 0 && info->op == VIRTIO_VSOCK_OP_RW)
> > > return pkt_len;
> > > ...
> > > }
> > >
> > > then it queues the packet for the TX worker calling .send_pkt()
> > > [vhost_transport_send_pkt() in the vhost_transport case]
> > >
> > > The main function executed by the TX worker is
> > > vhost_transport_do_send_pkt() that picks up a buffer from the virtqueue
> > > and it tries to copy the packet (up to 4K) on it. If the buffer
> > > allocated from the guest will be smaller then 4K, I think here it will
> > > be discarded with an error:
> > >
>
> I'm adding more lines to explain better.
>
> > > static void
> > > vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
> > > struct vhost_virtqueue *vq)
> > > {
> ...
>
> head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
> &out, &in, NULL, NULL);
>
> ...
>
> len = iov_length(&vq->iov[out], in);
> iov_iter_init(&iov_iter, READ, &vq->iov[out], in, len);
>
> nbytes = copy_to_iter(&pkt->hdr, sizeof(pkt->hdr), &iov_iter);
> if (nbytes != sizeof(pkt->hdr)) {
> virtio_transport_free_pkt(pkt);
> vq_err(vq, "Faulted on copying pkt hdr\n");
> break;
> }
>
> > > ...
> > > nbytes = copy_to_iter(pkt->buf, pkt->len, &iov_iter);
> >
> > isn't pck len the actual length though?
> >
>
> It is the length of the packet that we are copying in the guest RX
> buffers pointed by the iov_iter. The guest allocates an iovec with 2
> buffers, one for the header and one for the payload (4KB).
BTW at the moment that forces another kmalloc within virtio core. Maybe
vsock needs a flag to skip allocation in this case. Worth benchmarking.
See virtqueue_use_indirect which just does total_sg > 1.
>
> > > if (nbytes != pkt->len) {
> > > virtio_transport_free_pkt(pkt);
> > > vq_err(vq, "Faulted on copying pkt buf\n");
> > > break;
> > > }
> > > ...
> > > }
> > >
> > >
> > > This series changes this behavior since now we will split the packet in
> > > vhost_transport_do_send_pkt() depending on the buffer found in the
> > > virtqueue.
> > >
> > > We didn't change the buffer size in this series, so we still backward
> > > compatible, but if we will use buffers smaller than 4K, we should
> > > encounter the error described above.
So that's an implementation bug then? It made an assumption
of a 4K sized buffer? Or even PAGE_SIZE sized buffer?
> > >
> > > How do you suggest we proceed if we want to change the buffer size?
> > > Maybe adding a feature to "support any buffer size"?
> > >
> > > Thanks,
> > > Stefano
> >
> >
>
> --
^ permalink raw reply
* Re: [PATCH 2/2] Fix a NULL-ptr-deref bug in ath10k_usb_alloc_urb_from_pipe
From: Kalle Valo @ 2019-09-01 8:06 UTC (permalink / raw)
To: Guenter Roeck
Cc: Hui Peng, davem, Mathias Payer, ath10k, linux-wireless, netdev,
linux-kernel
In-Reply-To: <20190831213139.GA32507@roeck-us.net>
Guenter Roeck <linux@roeck-us.net> writes:
> Hi,
>
> On Sat, Aug 03, 2019 at 08:31:01PM -0400, Hui Peng wrote:
>> The `ar_usb` field of `ath10k_usb_pipe_usb_pipe` objects
>> are initialized to point to the containing `ath10k_usb` object
>> according to endpoint descriptors read from the device side, as shown
>> below in `ath10k_usb_setup_pipe_resources`:
>>
>> for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) {
>> endpoint = &iface_desc->endpoint[i].desc;
>>
>> // get the address from endpoint descriptor
>> pipe_num = ath10k_usb_get_logical_pipe_num(ar_usb,
>> endpoint->bEndpointAddress,
>> &urbcount);
>> ......
>> // select the pipe object
>> pipe = &ar_usb->pipes[pipe_num];
>>
>> // initialize the ar_usb field
>> pipe->ar_usb = ar_usb;
>> }
>>
>> The driver assumes that the addresses reported in endpoint
>> descriptors from device side to be complete. If a device is
>> malicious and does not report complete addresses, it may trigger
>> NULL-ptr-deref `ath10k_usb_alloc_urb_from_pipe` and
>> `ath10k_usb_free_urb_to_pipe`.
>>
>> This patch fixes the bug by preventing potential NULL-ptr-deref.
>>
>> Signed-off-by: Hui Peng <benquike@gmail.com>
>> Reported-by: Hui Peng <benquike@gmail.com>
>> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
>
> This patch fixes CVE-2019-15099, which has CVSS scores of 7.5 (CVSS 3.0)
> and 7.8 (CVSS 2.0). Yet, I don't find it in the upstream kernel or in Linux
> next.
>
> Is the patch going to be applied to the upstream kernel anytime soon ?
Same answer as in patch 1:
https://patchwork.kernel.org/patch/11074655/
--
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox