Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v6 net-next 14/19] ionic: Add initial ethtool support
From: Andrew Lunn @ 2019-09-01 19:52 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: Shannon Nelson, netdev, davem
In-Reply-To: <20190830151641.0aec4a3e@cakuba.netronome.com>

On Fri, Aug 30, 2019 at 03:16:41PM -0700, Jakub Kicinski wrote:
> On Fri, 30 Aug 2019 14:25:12 -0700, Shannon Nelson wrote:
> > On 8/29/19 4:10 PM, Jakub Kicinski wrote:
> > > On Thu, 29 Aug 2019 11:27:15 -0700, Shannon Nelson wrote:  
> > >> +static int ionic_get_module_eeprom(struct net_device *netdev,
> > >> +				   struct ethtool_eeprom *ee,
> > >> +				   u8 *data)
> > >> +{
> > >> +	struct ionic_lif *lif = netdev_priv(netdev);
> > >> +	struct ionic_dev *idev = &lif->ionic->idev;
> > >> +	struct ionic_xcvr_status *xcvr;
> > >> +	char tbuf[sizeof(xcvr->sprom)];
> > >> +	int count = 10;
> > >> +	u32 len;
> > >> +
> > >> +	/* The NIC keeps the module prom up-to-date in the DMA space
> > >> +	 * so we can simply copy the module bytes into the data buffer.
> > >> +	 */
> > >> +	xcvr = &idev->port_info->status.xcvr;
> > >> +	len = min_t(u32, sizeof(xcvr->sprom), ee->len);
> > >> +
> > >> +	do {
> > >> +		memcpy(data, xcvr->sprom, len);
> > >> +		memcpy(tbuf, xcvr->sprom, len);
> > >> +
> > >> +		/* Let's make sure we got a consistent copy */
> > >> +		if (!memcmp(data, tbuf, len))
> > >> +			break;
> > >> +
> > >> +	} while (--count);  
> > > Should this return an error if the image was never consistent?  
> > 
> > Sure, how about -EBUSY?
> 
> Or EAGAIN ? Not sure

ETIMEOUT?

EAGAIN would suggest it is a temporary problem. But if this fails
after 10 attempts, i would guess it is not a temporary problem, the
firmware is in deep trouble, or the SFP is loose in its cage, about to
fall out.

	 Andrew

^ permalink raw reply

* Re: [PATCH 2/2] Fix a NULL-ptr-deref bug in ath10k_usb_alloc_urb_from_pipe
From: Hui Peng @ 2019-09-01 19:45 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: kvalo, davem, Mathias Payer, ath10k, linux-wireless, netdev,
	linux-kernel
In-Reply-To: <20190831213139.GA32507@roeck-us.net>

On 8/31/19 5:31 PM, Guenter Roeck wrote:
> 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 ? If
> not, is there reason to believe that its severity may not be as high as the
> CVSS score indicates ?
The score was assigned by MITRE.
Same as previous ones, it is under review, once passed, it will be applied.
> Thanks,
> Guenter
>
>> ---
>>  drivers/net/wireless/ath/ath10k/usb.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/usb.c b/drivers/net/wireless/ath/ath10k/usb.c
>> index e1420f67f776..14d86627b47f 100644
>> --- a/drivers/net/wireless/ath/ath10k/usb.c
>> +++ b/drivers/net/wireless/ath/ath10k/usb.c
>> @@ -38,6 +38,10 @@ ath10k_usb_alloc_urb_from_pipe(struct ath10k_usb_pipe *pipe)
>>  	struct ath10k_urb_context *urb_context = NULL;
>>  	unsigned long flags;
>>  
>> +	/* bail if this pipe is not initialized */
>> +	if (!pipe->ar_usb)
>> +		return NULL;
>> +
>>  	spin_lock_irqsave(&pipe->ar_usb->cs_lock, flags);
>>  	if (!list_empty(&pipe->urb_list_head)) {
>>  		urb_context = list_first_entry(&pipe->urb_list_head,
>> @@ -55,6 +59,10 @@ static void ath10k_usb_free_urb_to_pipe(struct ath10k_usb_pipe *pipe,
>>  {
>>  	unsigned long flags;
>>  
>> +	/* bail if this pipe is not initialized */
>> +	if (!pipe->ar_usb)
>> +		return NULL;
>> +
>>  	spin_lock_irqsave(&pipe->ar_usb->cs_lock, flags);
>>  
>>  	pipe->urb_cnt++;
>> -- 
>> 2.22.0
>>

^ permalink raw reply

* Re: [PATCH net-next 00/10] net: dsa: mv88e6xxx: centralize SERDES IRQ handling
From: David Miller @ 2019-09-01 19:25 UTC (permalink / raw)
  To: vivien.didelot; +Cc: netdev, marek.behun, f.fainelli, andrew
In-Reply-To: <20190831201836.19957-1-vivien.didelot@gmail.com>

From: Vivien Didelot <vivien.didelot@gmail.com>
Date: Sat, 31 Aug 2019 16:18:26 -0400

> Following Marek's work on the abstraction of the SERDES lanes mapping, this
> series trades the .serdes_irq_setup and .serdes_irq_free callbacks for new
> .serdes_irq_mapping, .serdes_irq_enable and .serdes_irq_status operations.
> 
> This has the benefit to limit the various SERDES implementations to simple
> register accesses only; centralize the IRQ handling and mutex locking logic;
> as well as reducing boilerplate in the driver.

Looks good, series applied.

^ permalink raw reply

* Re: [PATCH net-next] net: hns3: remove set but not used variable 'qos'
From: David Miller @ 2019-09-01 19:13 UTC (permalink / raw)
  To: yuehaibing
  Cc: yisen.zhuang, salil.mehta, linyunsheng, liuzhongzhu,
	huangguangbin2, lipeng321, tanhuazhong, shenjian15, netdev,
	kernel-janitors, hulkci
In-Reply-To: <20190831122911.181336-1-yuehaibing@huawei.com>

From: YueHaibing <yuehaibing@huawei.com>
Date: Sat, 31 Aug 2019 12:29:11 +0000

> Fixes gcc '-Wunused-but-set-variable' warning:
> 
> drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c: In function 'hclge_restore_vlan_table':
> drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c:8016:18: warning:
>  variable 'qos' set but not used [-Wunused-but-set-variable]
> 
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Fixes: 70a214903da9 ("net: hns3: reduce the parameters of some functions")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Applied.

^ permalink raw reply

* Re: [PATCH][next] net: hns3: remove redundant assignment to pointer reg_info
From: David Miller @ 2019-09-01 19:12 UTC (permalink / raw)
  To: colin.king
  Cc: yisen.zhuang, salil.mehta, tanhuazhong, liuzhongzhu, netdev,
	kernel-janitors, linux-kernel
In-Reply-To: <20190831072949.7505-1-colin.king@canonical.com>

From: Colin King <colin.king@canonical.com>
Date: Sat, 31 Aug 2019 08:29:49 +0100

> From: Colin Ian King <colin.king@canonical.com>
> 
> Pointer reg_info 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.

^ permalink raw reply

* Re: [PATCH] net: seeq: Fix the function used to release some memory in an error handling path
From: David Miller @ 2019-09-01 19:11 UTC (permalink / raw)
  To: christophe.jaillet
  Cc: yuehaibing, tglx, gregkh, tbogendoerfer, netdev, linux-kernel,
	kernel-janitors
In-Reply-To: <20190831071751.1479-1-christophe.jaillet@wanadoo.fr>

From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Date: Sat, 31 Aug 2019 09:17:51 +0200

> In commit 99cd149efe82 ("sgiseeq: replace use of dma_cache_wback_inv"),
> a call to 'get_zeroed_page()' has been turned into a call to
> 'dma_alloc_coherent()'. Only the remove function has been updated to turn
> the corresponding 'free_page()' into 'dma_free_attrs()'.
> The error hndling path of the probe function has not been updated.
> 
> Fix it now.
> 
> Rename the corresponding label to something more in line.
> 
> Fixes: 99cd149efe82 ("sgiseeq: replace use of dma_cache_wback_inv")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>

Applied.

> If 'dma_alloc_coherent()' fails, maybe the message in printk could be
> improved. The comment above may also not be relevant.

Memory allocation failures already give a stack backtrack down deep in the
memory allocators, therefore printing messages at allocation call sites
are veboten.

^ permalink raw reply

* 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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox