* [PATCH V3 net 3/3] net: ena: fix packet's addresses for rx_offset feature
From: Shay Agroskin @ 2020-11-23 19:08 UTC (permalink / raw)
To: kuba, netdev
Cc: Shay Agroskin, dwmw, zorik, matua, saeedb, msw, aliguori, nafea,
gtzalik, netanel, alisaidi, benh, akiyano, sameehj, ndagan
In-Reply-To: <20201123190859.21298-1-shayagr@amazon.com>
This patch fixes two lines in which the rx_offset received by the device
wasn't taken into account:
- prefetch function:
In our driver the copied data would reside in
rx_info->page + rx_headroom + rx_offset
so the prefetch function is changed accordingly.
- setting page_offset to zero for descriptors > 1:
for every descriptor but the first, the rx_offset is zero. Hence
the page_offset value should be set to rx_headroom.
The previous implementation changed the value of rx_info after
the descriptor was added to the SKB (essentially providing wrong
page offset).
Fixes: 68f236df93a9 ("net: ena: add support for the rx offset feature")
Signed-off-by: Shay Agroskin <shayagr@amazon.com>
---
drivers/net/ethernet/amazon/ena/ena_netdev.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index ec0008ba7751..df1884d57d1a 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -908,10 +908,14 @@ static void ena_free_all_io_rx_resources(struct ena_adapter *adapter)
static int ena_alloc_rx_page(struct ena_ring *rx_ring,
struct ena_rx_buffer *rx_info, gfp_t gfp)
{
+ int headroom = rx_ring->rx_headroom;
struct ena_com_buf *ena_buf;
struct page *page;
dma_addr_t dma;
+ /* restore page offset value in case it has been changed by device */
+ rx_info->page_offset = headroom;
+
/* if previous allocated page is not used */
if (unlikely(rx_info->page))
return 0;
@@ -941,10 +945,9 @@ static int ena_alloc_rx_page(struct ena_ring *rx_ring,
"Allocate page %p, rx_info %p\n", page, rx_info);
rx_info->page = page;
- rx_info->page_offset = 0;
ena_buf = &rx_info->ena_buf;
- ena_buf->paddr = dma + rx_ring->rx_headroom;
- ena_buf->len = ENA_PAGE_SIZE - rx_ring->rx_headroom;
+ ena_buf->paddr = dma + headroom;
+ ena_buf->len = ENA_PAGE_SIZE - headroom;
return 0;
}
@@ -1356,7 +1359,8 @@ static struct sk_buff *ena_rx_skb(struct ena_ring *rx_ring,
/* save virt address of first buffer */
va = page_address(rx_info->page) + rx_info->page_offset;
- prefetch(va + NET_IP_ALIGN);
+
+ prefetch(va);
if (len <= rx_ring->rx_copybreak) {
skb = ena_alloc_skb(rx_ring, false);
@@ -1397,8 +1401,6 @@ static struct sk_buff *ena_rx_skb(struct ena_ring *rx_ring,
skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, rx_info->page,
rx_info->page_offset, len, ENA_PAGE_SIZE);
- /* The offset is non zero only for the first buffer */
- rx_info->page_offset = 0;
netif_dbg(rx_ring->adapter, rx_status, rx_ring->netdev,
"RX skb updated. len %d. data_len %d\n",
@@ -1517,8 +1519,7 @@ static int ena_xdp_handle_buff(struct ena_ring *rx_ring, struct xdp_buff *xdp)
int ret;
rx_info = &rx_ring->rx_buffer_info[rx_ring->ena_bufs[0].req_id];
- xdp->data = page_address(rx_info->page) +
- rx_info->page_offset + rx_ring->rx_headroom;
+ xdp->data = page_address(rx_info->page) + rx_info->page_offset;
xdp_set_data_meta_invalid(xdp);
xdp->data_hard_start = page_address(rx_info->page);
xdp->data_end = xdp->data + rx_ring->ena_bufs[0].len;
@@ -1585,8 +1586,9 @@ static int ena_clean_rx_irq(struct ena_ring *rx_ring, struct napi_struct *napi,
if (unlikely(ena_rx_ctx.descs == 0))
break;
+ /* First descriptor might have an offset set by the device */
rx_info = &rx_ring->rx_buffer_info[rx_ring->ena_bufs[0].req_id];
- rx_info->page_offset = ena_rx_ctx.pkt_offset;
+ rx_info->page_offset += ena_rx_ctx.pkt_offset;
netif_dbg(rx_ring->adapter, rx_status, rx_ring->netdev,
"rx_poll: q %d got packet from ena. descs #: %d l3 proto %d l4 proto %d hash: %x\n",
--
2.17.1
^ permalink raw reply related
* [PATCH V3 net 0/3] Fixes for ENA driver
From: Shay Agroskin @ 2020-11-23 19:08 UTC (permalink / raw)
To: kuba, netdev
Cc: Shay Agroskin, dwmw, zorik, matua, saeedb, msw, aliguori, nafea,
gtzalik, netanel, alisaidi, benh, akiyano, sameehj, ndagan
Hi all,
This series fixes some issues in the ENA driver:
- fix wrong data offset on machines that support rx offset
- work-around Intel iommu issue
- fix out of bound access when request id is wrong
Changes from previous version:
v1->v2: switched to using dma_set_mask_and_coherent() function
in second patch
v2->v3: Removed fourth patch from series (would be sent in future series)
Shay Agroskin (3):
net: ena: handle bad request id in ena_netdev
net: ena: set initial DMA width to avoid intel iommu issue
net: ena: fix packet's addresses for rx_offset feature
drivers/net/ethernet/amazon/ena/ena_eth_com.c | 3 +
drivers/net/ethernet/amazon/ena/ena_netdev.c | 80 +++++++------------
2 files changed, 33 insertions(+), 50 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH net-next v1 1/2] lan743x: clean up software_isr function
From: Sven Van Asbroeck @ 2020-11-23 19:15 UTC (permalink / raw)
To: Bryan Whitehead, Jakub Kicinski, David S Miller
Cc: Sven Van Asbroeck, Andrew Lunn, Microchip Linux Driver Support,
netdev, linux-kernel
From: Sven Van Asbroeck <thesven73@gmail.com>
For no apparent reason, this function reads the INT_STS register, and
checks if the software interrupt bit is set. These things have already
been carried out by this function's only caller.
Clean up by removing the redundant code.
Tested-by: Sven Van Asbroeck <thesven73@gmail.com> # lan7430
Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com>
---
Tree: git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git # f9e425e99b07
To: Bryan Whitehead <bryan.whitehead@microchip.com>
To: Jakub Kicinski <kuba@kernel.org>
To: "David S. Miller" <davem@davemloft.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
drivers/net/ethernet/microchip/lan743x_main.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index 87b6c59a1e03..bdc80098c240 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -140,18 +140,13 @@ static int lan743x_csr_init(struct lan743x_adapter *adapter)
return result;
}
-static void lan743x_intr_software_isr(void *context)
+static void lan743x_intr_software_isr(struct lan743x_adapter *adapter)
{
- struct lan743x_adapter *adapter = context;
struct lan743x_intr *intr = &adapter->intr;
- u32 int_sts;
- int_sts = lan743x_csr_read(adapter, INT_STS);
- if (int_sts & INT_BIT_SW_GP_) {
- /* disable the interrupt to prevent repeated re-triggering */
- lan743x_csr_write(adapter, INT_EN_CLR, INT_BIT_SW_GP_);
- intr->software_isr_flag = 1;
- }
+ /* disable the interrupt to prevent repeated re-triggering */
+ lan743x_csr_write(adapter, INT_EN_CLR, INT_BIT_SW_GP_);
+ intr->software_isr_flag = 1;
}
static void lan743x_tx_isr(void *context, u32 int_sts, u32 flags)
--
2.17.1
^ permalink raw reply related
* [PATCH net-next v1 2/2] lan743x: replace polling loop by wait_event_timeout()
From: Sven Van Asbroeck @ 2020-11-23 19:15 UTC (permalink / raw)
To: Bryan Whitehead, Jakub Kicinski, David S Miller
Cc: Sven Van Asbroeck, Andrew Lunn, Microchip Linux Driver Support,
netdev, linux-kernel
In-Reply-To: <20201123191529.14908-1-TheSven73@gmail.com>
From: Sven Van Asbroeck <thesven73@gmail.com>
The driver's ISR sends a 'software interrupt' event to the probe()
thread using the following method:
- probe(): write 0 to flag, enable s/w interrupt
- probe(): poll on flag, relax using usleep_range()
- ISR : write 1 to flag
Replace with wake_up() / wait_event_timeout(). Besides being easier
to get right, this abstraction has better timing and memory
consistency properties.
Tested-by: Sven Van Asbroeck <thesven73@gmail.com> # lan7430
Signed-off-by: Sven Van Asbroeck <thesven73@gmail.com>
---
To: Bryan Whitehead <bryan.whitehead@microchip.com>
To: Jakub Kicinski <kuba@kernel.org>
To: "David S. Miller" <davem@davemloft.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
Cc: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
drivers/net/ethernet/microchip/lan743x_main.c | 28 +++++++++----------
drivers/net/ethernet/microchip/lan743x_main.h | 3 +-
2 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index bdc80098c240..96d34b001198 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -146,7 +146,8 @@ static void lan743x_intr_software_isr(struct lan743x_adapter *adapter)
/* disable the interrupt to prevent repeated re-triggering */
lan743x_csr_write(adapter, INT_EN_CLR, INT_BIT_SW_GP_);
- intr->software_isr_flag = 1;
+ intr->software_isr_flag = true;
+ wake_up(&intr->software_isr_wq);
}
static void lan743x_tx_isr(void *context, u32 int_sts, u32 flags)
@@ -344,27 +345,22 @@ static irqreturn_t lan743x_intr_entry_isr(int irq, void *ptr)
static int lan743x_intr_test_isr(struct lan743x_adapter *adapter)
{
struct lan743x_intr *intr = &adapter->intr;
- int result = -ENODEV;
- int timeout = 10;
+ int ret;
- intr->software_isr_flag = 0;
+ intr->software_isr_flag = false;
- /* enable interrupt */
+ /* enable and activate test interrupt */
lan743x_csr_write(adapter, INT_EN_SET, INT_BIT_SW_GP_);
-
- /* activate interrupt here */
lan743x_csr_write(adapter, INT_SET, INT_BIT_SW_GP_);
- while ((timeout > 0) && (!(intr->software_isr_flag))) {
- usleep_range(1000, 20000);
- timeout--;
- }
- if (intr->software_isr_flag)
- result = 0;
+ ret = wait_event_timeout(intr->software_isr_wq,
+ intr->software_isr_flag,
+ msecs_to_jiffies(200));
- /* disable interrupts */
+ /* disable test interrupt */
lan743x_csr_write(adapter, INT_EN_CLR, INT_BIT_SW_GP_);
- return result;
+
+ return ret > 0 ? 0 : -ENODEV;
}
static int lan743x_intr_register_isr(struct lan743x_adapter *adapter,
@@ -538,6 +534,8 @@ static int lan743x_intr_open(struct lan743x_adapter *adapter)
flags |= LAN743X_VECTOR_FLAG_SOURCE_ENABLE_R2C;
}
+ init_waitqueue_head(&intr->software_isr_wq);
+
ret = lan743x_intr_register_isr(adapter, 0, flags,
INT_BIT_ALL_RX_ | INT_BIT_ALL_TX_ |
INT_BIT_ALL_OTHER_,
diff --git a/drivers/net/ethernet/microchip/lan743x_main.h b/drivers/net/ethernet/microchip/lan743x_main.h
index a536f4a4994d..b92864913e6c 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.h
+++ b/drivers/net/ethernet/microchip/lan743x_main.h
@@ -616,7 +616,8 @@ struct lan743x_intr {
int number_of_vectors;
bool using_vectors;
- int software_isr_flag;
+ bool software_isr_flag;
+ wait_queue_head_t software_isr_wq;
};
#define LAN743X_MAX_FRAME_SIZE (9 * 1024)
--
2.17.1
^ permalink raw reply related
* Re: netconsole deadlock with virtnet
From: Jakub Kicinski @ 2020-11-23 19:21 UTC (permalink / raw)
To: Steven Rostedt
Cc: Leon Romanovsky, Jason Wang, Sergey Senozhatsky,
Michael S. Tsirkin, Petr Mladek, John Ogness, virtualization,
Amit Shah, Itay Aveksis, Ran Rozenstein, netdev
In-Reply-To: <20201123140934.38748be3@gandalf.local.home>
On Mon, 23 Nov 2020 14:09:34 -0500 Steven Rostedt wrote:
> On Mon, 23 Nov 2020 10:52:52 -0800
> Jakub Kicinski <kuba@kernel.org> wrote:
>
> > On Mon, 23 Nov 2020 09:31:28 -0500 Steven Rostedt wrote:
> > > On Mon, 23 Nov 2020 13:08:55 +0200
> > > Leon Romanovsky <leon@kernel.org> wrote:
> > >
> > >
> > > > [ 10.028024] Chain exists of:
> > > > [ 10.028025] console_owner --> target_list_lock --> _xmit_ETHER#2
> > >
> > > Note, the problem is that we have a location that grabs the xmit_lock while
> > > holding target_list_lock (and possibly console_owner).
> >
> > Well, it try_locks the xmit_lock. Does lockdep understand try-locks?
> >
> > (not that I condone the shenanigans that are going on here)
>
> Does it?
>
> virtnet_poll_tx() {
> __netif_tx_lock() {
> spin_lock(&txq->_xmit_lock);
Umpf. Right. I was looking at virtnet_poll_cleantx()
> That looks like we can have:
>
>
> CPU0 CPU1
> ---- ----
> lock(xmit_lock)
>
> lock(console)
> lock(target_list_lock)
> __netif_tx_lock()
> lock(xmit_lock);
>
> [BLOCKED]
>
> <interrupt>
> lock(console)
>
> [BLOCKED]
>
>
>
> DEADLOCK.
>
>
> So where is the trylock here?
>
> Perhaps you need the trylock in virtnet_poll_tx()?
That could work. Best if we used normal lock if !!budget, and trylock
when budget is 0. But maybe that's too hairy.
I'm assuming all this trickiness comes from virtqueue_get_buf() needing
locking vs the TX path? It's pretty unusual for the completion path to
need locking vs xmit path.
^ permalink raw reply
* [PATCH net v2 0/3] ibmvnic: null pointer dereference
From: Lijun Pan @ 2020-11-23 19:35 UTC (permalink / raw)
To: netdev; +Cc: sukadev, drt, Lijun Pan
Fix two NULL pointer dereference crash issues.
Improve module removal procedure.
In v2, we split v1 into 3 sets according to patch dependencies so that
individual author can rework on them during the coming holiday.
1-11 as a set since they are dependent and most of them are Dany's.
12-14 as a set since they are independent of 1-11.
15 to be sent to net-next.
This series come from 12/15 13/15 14/15 of v1's "ibmvnic: assorted bug
fixes".
Lijun Pan (3):
ibmvnic: fix NULL pointer dereference in reset_sub_crq_queues
ibmvnic: fix NULL pointer dereference in ibmvic_reset_crq
ibmvnic: enhance resetting status check during module exit
drivers/net/ethernet/ibm/ibmvnic.c | 9 +++++++--
drivers/net/ethernet/ibm/ibmvnic.h | 3 +--
2 files changed, 8 insertions(+), 4 deletions(-)
--
2.23.0
^ permalink raw reply
* [PATCH net v2 1/3] ibmvnic: fix NULL pointer dereference in reset_sub_crq_queues
From: Lijun Pan @ 2020-11-23 19:35 UTC (permalink / raw)
To: netdev; +Cc: sukadev, drt, Lijun Pan
In-Reply-To: <20201123193547.57225-1-ljp@linux.ibm.com>
adapter->tx_scrq and adapter->rx_scrq could be NULL if the previous reset
did not complete after freeing sub crqs. Check for NULL before
dereferencing them.
Snippet of call trace:
ibmvnic 30000006 env6: Releasing sub-CRQ
ibmvnic 30000006 env6: Releasing CRQ
...
ibmvnic 30000006 env6: Got Control IP offload Response
ibmvnic 30000006 env6: Re-setting tx_scrq[0]
BUG: Kernel NULL pointer dereference on read at 0x00000000
Faulting instruction address: 0xc008000003dea7cc
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in: rpadlpar_io rpaphp xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT nf_reject_ipv4 nft_compat nft_counter nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables xsk_diag tcp_diag udp_diag raw_diag inet_diag unix_diag af_packet_diag netlink_diag tun bridge stp llc rfkill sunrpc pseries_rng xts vmx_crypto uio_pdrv_genirq uio binfmt_misc ip_tables xfs libcrc32c sd_mod t10_pi sg ibmvscsi ibmvnic ibmveth scsi_transport_srp dm_mirror dm_region_hash dm_log dm_mod
CPU: 80 PID: 1856 Comm: kworker/80:2 Tainted: G W 5.8.0+ #4
Workqueue: events __ibmvnic_reset [ibmvnic]
NIP: c008000003dea7cc LR: c008000003dea7bc CTR: 0000000000000000
REGS: c0000007ef7db860 TRAP: 0380 Tainted: G W (5.8.0+)
MSR: 800000000280b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR: 28002422 XER: 0000000d
CFAR: c000000000bd9520 IRQMASK: 0
GPR00: c008000003dea7bc c0000007ef7dbaf0 c008000003df7400 c0000007fa26ec00
GPR04: c0000007fcd0d008 c0000007fcd96350 0000000000000027 c0000007fcd0d010
GPR08: 0000000000000023 0000000000000000 0000000000000000 0000000000000000
GPR12: 0000000000002000 c00000001ec18e00 c0000000001982f8 c0000007bad6e840
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000000 0000000000000000 0000000000000000 fffffffffffffef7
GPR24: 0000000000000402 c0000007fa26f3a8 0000000000000003 c00000016f8ec048
GPR28: 0000000000000000 0000000000000000 0000000000000000 c0000007fa26ec00
NIP [c008000003dea7cc] ibmvnic_reset_init+0x15c/0x258 [ibmvnic]
LR [c008000003dea7bc] ibmvnic_reset_init+0x14c/0x258 [ibmvnic]
Call Trace:
[c0000007ef7dbaf0] [c008000003dea7bc] ibmvnic_reset_init+0x14c/0x258 [ibmvnic] (unreliable)
[c0000007ef7dbb80] [c008000003de8860] __ibmvnic_reset+0x408/0x970 [ibmvnic]
[c0000007ef7dbc50] [c00000000018b7cc] process_one_work+0x2cc/0x800
[c0000007ef7dbd20] [c00000000018bd78] worker_thread+0x78/0x520
[c0000007ef7dbdb0] [c0000000001984c4] kthread+0x1d4/0x1e0
[c0000007ef7dbe20] [c00000000000cea8] ret_from_kernel_thread+0x5c/0x74
Fixes: 57a49436f4e8 ("ibmvnic: Reset sub-crqs during driver reset")
Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
---
v2: add fix tag
remove verbose printout, return silently.
reset function itself will handle the error return
drivers/net/ethernet/ibm/ibmvnic.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 47446e5f8ec5..5f4fdd13184a 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2930,6 +2930,9 @@ static int reset_sub_crq_queues(struct ibmvnic_adapter *adapter)
{
int i, rc;
+ if (!adapter->tx_scrq || !adapter->rx_scrq)
+ return -EINVAL;
+
for (i = 0; i < adapter->req_tx_queues; i++) {
netdev_dbg(adapter->netdev, "Re-setting tx_scrq[%d]\n", i);
rc = reset_one_sub_crq_queue(adapter, adapter->tx_scrq[i]);
--
2.23.0
^ permalink raw reply related
* [PATCH net v2 2/3] ibmvnic: fix NULL pointer dereference in ibmvic_reset_crq
From: Lijun Pan @ 2020-11-23 19:35 UTC (permalink / raw)
To: netdev; +Cc: sukadev, drt, Lijun Pan
In-Reply-To: <20201123193547.57225-1-ljp@linux.ibm.com>
crq->msgs could be NULL if the previous reset did not complete after
freeing crq->msgs. Check for NULL before dereferencing them.
Snippet of call trace:
...
ibmvnic 30000003 env3 (unregistering): Releasing sub-CRQ
ibmvnic 30000003 env3 (unregistering): Releasing CRQ
BUG: Kernel NULL pointer dereference on read at 0x00000000
Faulting instruction address: 0xc0000000000c1a30
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in: ibmvnic(E-) rpadlpar_io rpaphp xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT nf_reject_ipv4 nft_compat nft_counter nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables xsk_diag tcp_diag udp_diag tun raw_diag inet_diag unix_diag bridge af_packet_diag netlink_diag stp llc rfkill sunrpc pseries_rng xts vmx_crypto uio_pdrv_genirq uio binfmt_misc ip_tables xfs libcrc32c sd_mod t10_pi sg ibmvscsi ibmveth scsi_transport_srp dm_mirror dm_region_hash dm_log dm_mod [last unloaded: ibmvnic]
CPU: 20 PID: 8426 Comm: kworker/20:0 Tainted: G E 5.10.0-rc1+ #12
Workqueue: events __ibmvnic_reset [ibmvnic]
NIP: c0000000000c1a30 LR: c008000001b00c18 CTR: 0000000000000400
REGS: c00000000d05b7a0 TRAP: 0380 Tainted: G E (5.10.0-rc1+)
MSR: 800000000280b033 <SF,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR: 44002480 XER: 20040000
CFAR: c0000000000c19ec IRQMASK: 0
GPR00: 0000000000000400 c00000000d05ba30 c008000001b17c00 0000000000000000
GPR04: 0000000000000000 0000000000000000 0000000000000000 00000000000001e2
GPR08: 000000000001f400 ffffffffffffd950 0000000000000000 c008000001b0b280
GPR12: c0000000000c19c8 c00000001ec72e00 c00000000019a778 c00000002647b440
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: 0000000000000006 0000000000000001 0000000000000003 0000000000000002
GPR24: 0000000000001000 c008000001b0d570 0000000000000005 c00000007ab5d550
GPR28: c00000007ab5c000 c000000032fcf848 c00000007ab5cc00 c000000032fcf800
NIP [c0000000000c1a30] memset+0x68/0x104
LR [c008000001b00c18] ibmvnic_reset_crq+0x70/0x110 [ibmvnic]
Call Trace:
[c00000000d05ba30] [0000000000000800] 0x800 (unreliable)
[c00000000d05bab0] [c008000001b0a930] do_reset.isra.40+0x224/0x634 [ibmvnic]
[c00000000d05bb80] [c008000001b08574] __ibmvnic_reset+0x17c/0x3c0 [ibmvnic]
[c00000000d05bc50] [c00000000018d9ac] process_one_work+0x2cc/0x800
[c00000000d05bd20] [c00000000018df58] worker_thread+0x78/0x520
[c00000000d05bdb0] [c00000000019a934] kthread+0x1c4/0x1d0
[c00000000d05be20] [c00000000000d5d0] ret_from_kernel_thread+0x5c/0x6c
Fixes: 032c5e82847a ("Driver for IBM System i/p VNIC protocol")
Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
---
v2: add fix tag.
remove verbose printout, return silently.
reset function itself will handle the error return.
drivers/net/ethernet/ibm/ibmvnic.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 5f4fdd13184a..91b7a383debf 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -5049,6 +5049,9 @@ static int ibmvnic_reset_crq(struct ibmvnic_adapter *adapter)
} while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
/* Clean out the queue */
+ if (!crq->msgs)
+ return -EINVAL;
+
memset(crq->msgs, 0, PAGE_SIZE);
crq->cur = 0;
crq->active = false;
--
2.23.0
^ permalink raw reply related
* [PATCH net v2 3/3] ibmvnic: enhance resetting status check during module exit
From: Lijun Pan @ 2020-11-23 19:35 UTC (permalink / raw)
To: netdev; +Cc: sukadev, drt, Lijun Pan
In-Reply-To: <20201123193547.57225-1-ljp@linux.ibm.com>
Based on the discussion with Sukadev Bhattiprolu and Dany Madden,
we believe that checking adapter->resetting bit is preferred
since RESETTING state flag is not as strict as resetting bit.
RESETTING state flag is removed since it is verbose now.
Fixes: 7d7195a026ba ("ibmvnic: Do not process device remove during device reset")
Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
---
v2: add fix tag
drivers/net/ethernet/ibm/ibmvnic.c | 3 +--
drivers/net/ethernet/ibm/ibmvnic.h | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 91b7a383debf..5cb4cfabe2de 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2251,7 +2251,6 @@ static void __ibmvnic_reset(struct work_struct *work)
if (!saved_state) {
reset_state = adapter->state;
- adapter->state = VNIC_RESETTING;
saved_state = true;
}
spin_unlock_irqrestore(&adapter->state_lock, flags);
@@ -5357,7 +5356,7 @@ static int ibmvnic_remove(struct vio_dev *dev)
unsigned long flags;
spin_lock_irqsave(&adapter->state_lock, flags);
- if (adapter->state == VNIC_RESETTING) {
+ if (test_bit(0, &adapter->resetting)) {
spin_unlock_irqrestore(&adapter->state_lock, flags);
return -EBUSY;
}
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
index d15866cbc2a6..950f439bed32 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -943,8 +943,7 @@ enum vnic_state {VNIC_PROBING = 1,
VNIC_CLOSING,
VNIC_CLOSED,
VNIC_REMOVING,
- VNIC_REMOVED,
- VNIC_RESETTING};
+ VNIC_REMOVED};
enum ibmvnic_reset_reason {VNIC_RESET_FAILOVER = 1,
VNIC_RESET_MOBILITY,
--
2.23.0
^ permalink raw reply related
* Re: [EXT] Re: [PATCH v3] aquantia: Remove the build_skb path
From: Maciej Fijalkowski @ 2020-11-23 19:28 UTC (permalink / raw)
To: Igor Russkikh
Cc: Ramsay, Lincoln, Florian Westphal, David S. Miller,
Jakub Kicinski, netdev, Dmitry Bogdanov [C]
In-Reply-To: <2fbb195a-a1b5-cec0-1ba1-bf45efc0ad24@marvell.com>
On Fri, Nov 20, 2020 at 11:18:34AM +0300, Igor Russkikh wrote:
>
>
> On 20/11/2020 1:49 am, Maciej Fijalkowski wrote:
> > External Email
> >
> > ----------------------------------------------------------------------
> > On Thu, Nov 19, 2020 at 10:34:48PM +0000, Ramsay, Lincoln wrote:
> >> When performing IPv6 forwarding, there is an expectation that SKBs
> >> will have some headroom. When forwarding a packet from the aquantia
> >> driver, this does not always happen, triggering a kernel warning.
> >>
> >> The build_skb path fails to allow for an SKB header, but the hardware
> >> buffer it is built around won't allow for this anyway. Just always use
> > the
> >> slower codepath that copies memory into an allocated SKB.
> >>
> >> Signed-off-by: Lincoln Ramsay <lincoln.ramsay@opengear.com>
> >> ---
> >
> > (Next time please include in the subject the tree that you're targetting
> > the patch)
> >
> > I feel like it's only a workaround, not a real solution. On previous
> > thread Igor says:
> >
> > "The limitation here is we can't tell HW on granularity less than 1K."
> >
> > Are you saying that the minimum headroom that we could provide is 1k?
>
> We can tell HW to place packets with 4 bytes granularity addresses, but the
> problem is the length granularity of this buffer - 1K.
>
> This means we can do as Ramsay initially suggested - just offset the packet
> placement. But then we have to guarantee that 1K after this offset is
> available to HW.
Ok, I see, thanks for clarifying.
>
> Since normal layout is 1400 packets - we do use 2K (half page) for each packet.
What is 'normal layout is 1400 packets' ? Didn't you mean the 1500 byte
standard MTU? So this is what you've been trying to tell me - that for
1500 byte mtu and 1k HW granularity you need to provide to HW 2k of
contiguous space, correct?
> This way we reuse each allocated page for at least two packets (and putting
> skb_shared into the remaining 512b).
I don't think I follow that. I thought that 2k needs to be exclusive for
HW and now you're saying that for remaining 512 bytes you can do whatever
you want.
If that's true then I think you can have build_skb support and I don't see
that 1k granularity as a limitation.
>
> Obviously we may allocate 4K page for a single packet, and tell HW that it can
> use 3K for data. This'll give 1K headroom. Quite an overload - assuming IMIX
> is of 0.5K - 1.4K..
>
> Of course that depends on a usecase. If you know all your traffic is 16K
> jumbos - putting 1K headroom is very small overhead on memory usage.
>
> > Maybe put more pressure on memory side and pull in order-1 pages, provide
> > this big headroom and tailroom for skb_shared_info and use build_skb by
> > default? With standard 1500 byte MTU.
> I know many customers do consider AQC chips in near embedded environments
> (routers, etc). They really do care about memories. So that could be risky.
We have a knob that is controlled by ethtool's priv flag so you can change
the memory model and pull the build_skb out of the picture. Just FYI.
>
> > This issue would pop up again if this driver would like to support XDP
> > where 256 byte headroom will have to be provided.
>
> Actually it already popped. Thats one of the reasons I'm delaying with xdp
> patch series for this driver.
>
> I think the best tradeoff here would be allocating order 1 or 2 pages (i.e. 8K
> or 16K), and reuse the page for multiple placements of 2K XDP packets:
>
> (256+2048)*3 = 6912 (1K overhead for each 3 packets)
>
> (256+2048)*7 = 16128 (200b overhead over 7 packets)
And for XDP_PASS you would use build_skb? Then tailroom needs to be
provided.
>
> Regards,
> Igor
>
>
>
^ permalink raw reply
* Re: [PATCH net-next v4 2/5] net/lapb: support netdev events
From: Jakub Kicinski @ 2020-11-23 19:36 UTC (permalink / raw)
To: Xie He
Cc: Martin Schiller, Andrew Hendry, David S. Miller, Linux X25,
Linux Kernel Network Developers, LKML
In-Reply-To: <CAJht_EPtPDOSYfwc=9trBMdzLw4BbTzJbGvaEgWoyiy2624Q+Q@mail.gmail.com>
On Mon, 23 Nov 2020 03:17:54 -0800 Xie He wrote:
> On Mon, Nov 23, 2020 at 2:38 AM Martin Schiller <ms@dev.tdt.de> wrote:
> > Well, one could argue that we would have to repair these drivers, but I
> > don't think that will get us anywhere.
>
> Yeah... One problem I see with the Linux project is the lack of
> docs/specs. Often we don't know what is right and what is wrong.
More of a historic thing than a requirement AFAIK. Some software
devices, e.g. loopback will not generate carrier events. But in this
case looks like all the devices Martin wants to handle are lapb.
> > From this point of view it will be the best to handle the NETDEV_UP in
> > the lapb event handler and establish the link analog to the
> > NETDEV_CHANGE event if the carrier is UP.
>
> Thanks! This way we can make sure LAPB would automatically connect in
> all situations.
>
> Since we'll have a netif_carrier_ok check in NETDEV_UP handing, it
> might make the code look prettier to also have a netif_carrier_ok
> check in NETDEV_GOING_DOWN handing (for symmetry). Just a suggestion.
> You can do whatever looks good to you :)
Xie other than this the patches look good to you?
Martin should I expect a respin to follow Xie's suggestion
or should I apply v4?
^ permalink raw reply
* Re: [PATCH net] net: openvswitch: fix TTL decrement action netlink message format
From: Matteo Croce @ 2020-11-23 19:36 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Eelco Chaudron, netdev, David Miller, dev, Pravin B Shelar,
bindiyakurle, Ilya Maximets
In-Reply-To: <20201120131228.489c3b52@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
On Fri, Nov 20, 2020 at 10:12 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 19 Nov 2020 04:04:04 -0500 Eelco Chaudron wrote:
> > Currently, the openvswitch module is not accepting the correctly formated
> > netlink message for the TTL decrement action. For both setting and getting
> > the dec_ttl action, the actions should be nested in the
> > OVS_DEC_TTL_ATTR_ACTION attribute as mentioned in the openvswitch.h uapi.
>
> IOW this change will not break any known user space, correct?
>
> But existing OvS user space already expects it to work like you
> make it work now?
>
> What's the harm in leaving it as is?
>
> > Fixes: 744676e77720 ("openvswitch: add TTL decrement action")
> > Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
>
> Can we get a review from OvS folks? Matteo looks good to you (as the
> original author)?
>
Hi,
I think that the userspace still has to implement the dec_ttl action;
by now dec_ttl is implemented with set_ttl().
So there is no breakage yet.
Eelco, with this fix we will encode the netlink attribute in the same
way for the kernel and netdev datapath?
If so, go for it.
> > - err = __ovs_nla_copy_actions(net, attr, key, sfa, eth_type,
> > + err = __ovs_nla_copy_actions(net, actions, key, sfa, eth_type,
> > vlan_tci, mpls_label_count, log);
> > if (err)
> > return err;
>
> You're not canceling any nests on error, I assume this is normal.
>
> > + add_nested_action_end(*sfa, action_start);
> > add_nested_action_end(*sfa, start);
> > return 0;
> > }
>
--
per aspera ad upstream
^ permalink raw reply
* Re: [PATCH net 00/15] ibmvnic: assorted bug fixes
From: ljp @ 2020-11-23 19:38 UTC (permalink / raw)
To: Lijun Pan; +Cc: netdev, sukadev, drt
In-Reply-To: <20201120224049.46933-1-ljp@linux.ibm.com>
On 2020-11-20 16:40, Lijun Pan wrote:
> Assorted fixes and improvements for ibmvnic bugs.
>
> Dany Madden (9):
> ibmvnic: handle inconsistent login with reset
> ibmvnic: process HMC disable command
> ibmvnic: stop free_all_rwi on failed reset
> ibmvnic: remove free_all_rwi function
> ibmvnic: avoid memset null scrq msgs
> ibmvnic: restore adapter state on failed reset
> ibmvnic: send_login should check for crq errors
> ibmvnic: no reset timeout for 5 seconds after reset
> ibmvnic: reduce wait for completion time
>
> Lijun Pan (3):
> ibmvnic: fix NULL pointer dereference in reset_sub_crq_queues
> ibmvnic: fix NULL pointer dereference in ibmvic_reset_crq
> ibmvnic: enhance resetting status check during module exit
>
> Sukadev Bhattiprolu (3):
> ibmvnic: delay next reset if hard reset failed
> ibmvnic: track pending login
> ibmvnic: add some debugs
>
> drivers/net/ethernet/ibm/ibmvnic.c | 246 +++++++++++++++++++++--------
> drivers/net/ethernet/ibm/ibmvnic.h | 9 +-
> 2 files changed, 183 insertions(+), 72 deletions(-)
In v2, we will split to 3 sets according to patch dependencies so that
the
individual author can re-work on them during the coming holiday season.
1-11 as a set since they are dependent and most of them are Dany's
patches
12-14 as a set since they are independent of 1-11.
15 to be sent to net-next.
Thanks,
Lijun
^ permalink raw reply
* Re: [PATCH net 02/15] ibmvnic: process HMC disable command
From: Jakub Kicinski @ 2020-11-23 19:43 UTC (permalink / raw)
To: drt; +Cc: Lijun Pan, netdev, sukadev, drt
In-Reply-To: <aa10c3fad62841df56a6185b3b267ca9@linux.vnet.ibm.com>
On Sun, 22 Nov 2020 07:12:38 -0800 drt wrote:
> On 2020-11-21 15:36, Jakub Kicinski wrote:
> > On Fri, 20 Nov 2020 16:40:36 -0600 Lijun Pan wrote:
> >> From: Dany Madden <drt@linux.ibm.com>
> >>
> >> Currently ibmvnic does not support the disable vnic command from the
> >> Hardware Management Console. This patch enables ibmvnic to process
> >> CRQ message 0x07, disable vnic adapter.
> >
> > What user-visible problem does this one solve?
> This allows HMC to disconnect a Linux client from the network if the
> vNIC adapter is misbehaving and/or sending malicious traffic. The effect
> is the same as when a sysadmin sets a link down (ibmvnic_close()) on the
> Linux client. This patch extends this ability to the HMC.
Okay, sounds to me like net-next material, then.
IIUC we don't need to fix this ASAP and backport to stable.
^ permalink raw reply
* Re: [PATCH net 15/15] ibmvnic: add some debugs
From: Sukadev Bhattiprolu @ 2020-11-23 19:48 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Lijun Pan, netdev, drt
In-Reply-To: <20201121154549.10f6fb7d@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Jakub Kicinski [kuba@kernel.org] wrote:
> On Fri, 20 Nov 2020 16:40:49 -0600 Lijun Pan wrote:
> > From: Sukadev Bhattiprolu <sukadev@linux.ibm.com>
> >
> > We sometimes run into situations where a soft/hard reset of the adapter
> > takes a long time or fails to complete. Having additional messages that
> > include important adapter state info will hopefully help understand what
> > is happening, reduce the guess work and minimize requests to reproduce
> > problems with debug patches.
>
> This doesn't qualify as a bug fix, please send it to net-next.
Ok.
>
> > + netdev_err(adapter->netdev,
> > + "[S:%d FRR:%d WFR:%d] Done processing resets\n",
> > + adapter->state, adapter->force_reset_recovery,
> > + adapter->wait_for_reset);
>
> Does reset only happen as a result of an error? Should this be a
> netdev_info() instead?
It is an informational message, will change to netdev_info().
Thanks,
Sukadev
^ permalink raw reply
* Re: [PATCH 000/141] Fix fall-through warnings for Clang
From: Jason Gunthorpe @ 2020-11-23 20:03 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: linux-kernel, alsa-devel, amd-gfx, bridge, ceph-devel,
cluster-devel, coreteam, devel, dm-devel, drbd-dev, dri-devel,
GR-everest-linux-l2, GR-Linux-NIC-Dev, intel-gfx, intel-wired-lan,
keyrings, linux1394-devel, linux-acpi, linux-afs,
linux-arm-kernel, linux-arm-msm, linux-atm-general, linux-block,
linux-can, linux-cifs, linux-crypto, linux-decnet-user,
linux-ext4, linux-fbdev, linux-geode, linux-gpio, linux-hams,
linux-hwmon, linux-i3c, linux-ide, linux-iio, linux-input,
linux-integrity, linux-mediatek, linux-media, linux-mmc, linux-mm,
linux-mtd, linux-nfs, linux-rdma, linux-renesas-soc, linux-scsi,
linux-sctp, linux-security-module, linux-stm32, linux-usb,
linux-watchdog, linux-wireless, netdev, netfilter-devel, nouveau,
op-tee, oss-drivers, patches, rds-devel, reiserfs-devel,
samba-technical, selinux, target-devel, tipc-discussion,
usb-storage, virtualization, wcn36xx, x86, xen-devel,
linux-hardening, Nick Desaulniers, Nathan Chancellor,
Miguel Ojeda, Joe Perches, Kees Cook
In-Reply-To: <cover.1605896059.git.gustavoars@kernel.org>
On Fri, Nov 20, 2020 at 12:21:39PM -0600, Gustavo A. R. Silva wrote:
> IB/hfi1: Fix fall-through warnings for Clang
> IB/mlx4: Fix fall-through warnings for Clang
> IB/qedr: Fix fall-through warnings for Clang
> RDMA/mlx5: Fix fall-through warnings for Clang
I picked these four to the rdma tree, thanks
Jason
^ permalink raw reply
* [PATCH net-next 01/17] keys: Provide the original description to the key preparser
From: David Howells @ 2020-11-23 20:10 UTC (permalink / raw)
To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <160616220405.830164.2239716599743995145.stgit@warthog.procyon.org.uk>
Provide the proposed description (add key) or the original description
(update/instantiate key) when preparsing a key so that the key type can
validate it against the data.
This is important for rxrpc server keys as we need to check that they have
the right amount of key material present - and it's better to do that when
the key is loaded rather than deep in trying to process a response packet.
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
cc: keyrings@vger.kernel.org
---
include/linux/key-type.h | 1 +
security/keys/key.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/include/linux/key-type.h b/include/linux/key-type.h
index 2ab2d6d6aeab..7d985a1dfe4a 100644
--- a/include/linux/key-type.h
+++ b/include/linux/key-type.h
@@ -29,6 +29,7 @@ struct kernel_pkey_params;
* clear the contents.
*/
struct key_preparsed_payload {
+ const char *orig_description; /* Actual or proposed description (maybe NULL) */
char *description; /* Proposed key description (or NULL) */
union key_payload payload; /* Proposed payload */
const void *data; /* Raw data */
diff --git a/security/keys/key.c b/security/keys/key.c
index e282c6179b21..ebe752b137aa 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -504,6 +504,7 @@ int key_instantiate_and_link(struct key *key,
int ret;
memset(&prep, 0, sizeof(prep));
+ prep.orig_description = key->description;
prep.data = data;
prep.datalen = datalen;
prep.quotalen = key->type->def_datalen;
@@ -854,6 +855,7 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
goto error_put_type;
memset(&prep, 0, sizeof(prep));
+ prep.orig_description = description;
prep.data = payload;
prep.datalen = plen;
prep.quotalen = index_key.type->def_datalen;
^ permalink raw reply related
* [PATCH net-next 03/17] rxrpc: List the held token types in the key description in /proc/keys
From: David Howells @ 2020-11-23 20:10 UTC (permalink / raw)
To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <160616220405.830164.2239716599743995145.stgit@warthog.procyon.org.uk>
When viewing an rxrpc-type key through /proc/keys, display a list of held
token types.
Signed-off-by: David Howells <dhowells@redhat.com>
---
net/rxrpc/key.c | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/net/rxrpc/key.c b/net/rxrpc/key.c
index fb4d2a2fca02..197b4cf46b64 100644
--- a/net/rxrpc/key.c
+++ b/net/rxrpc/key.c
@@ -31,6 +31,7 @@ static void rxrpc_free_preparse_s(struct key_preparsed_payload *);
static void rxrpc_destroy(struct key *);
static void rxrpc_destroy_s(struct key *);
static void rxrpc_describe(const struct key *, struct seq_file *);
+static void rxrpc_describe_s(const struct key *, struct seq_file *);
static long rxrpc_read(const struct key *, char *, size_t);
/*
@@ -61,7 +62,7 @@ struct key_type key_type_rxrpc_s = {
.free_preparse = rxrpc_free_preparse_s,
.instantiate = generic_key_instantiate,
.destroy = rxrpc_destroy_s,
- .describe = rxrpc_describe,
+ .describe = rxrpc_describe_s,
};
/*
@@ -494,6 +495,32 @@ static void rxrpc_destroy_s(struct key *key)
* describe the rxrpc key
*/
static void rxrpc_describe(const struct key *key, struct seq_file *m)
+{
+ const struct rxrpc_key_token *token;
+ const char *sep = ": ";
+
+ seq_puts(m, key->description);
+
+ for (token = key->payload.data[0]; token; token = token->next) {
+ seq_puts(m, sep);
+
+ switch (token->security_index) {
+ case RXRPC_SECURITY_RXKAD:
+ seq_puts(m, "ka");
+ break;
+ default: /* we have a ticket we can't encode */
+ seq_printf(m, "%u", token->security_index);
+ break;
+ }
+
+ sep = " ";
+ }
+}
+
+/*
+ * describe the rxrpc server key
+ */
+static void rxrpc_describe_s(const struct key *key, struct seq_file *m)
{
seq_puts(m, key->description);
}
^ permalink raw reply related
* [PATCH net 00/17] rxrpc: Prelude to gssapi support
From: David Howells @ 2020-11-23 20:10 UTC (permalink / raw)
To: netdev; +Cc: dhowells, linux-afs, linux-kernel
Here are some patches that do some reorganisation of the security class
handling in rxrpc to allow implementation of the RxGK security class that
will allow AF_RXRPC to use GSSAPI-negotiated tokens and better crypto. The
RxGK security class is not included in this patchset.
It does the following things:
(1) Add a keyrings patch to provide the original key description, as
provided to add_key(), to the payload preparser so that it can
interpret the content on that basis. Unfortunately, the rxrpc_s key
type wasn't written to interpret its payload as anything other than a
string of bytes comprising a key, but for RxGK, more information is
required as multiple Kerberos enctypes are supported.
(2) Remove the rxk5 security class key parsing. The rxk5 class never got
rolled out in OpenAFS and got replaced with rxgk.
(3) Support the creation of rxrpc keys with multiple tokens of different
types. If some types are not supported, the ENOPKG error is
suppressed if at least one other token's type is supported.
(4) Punt the handling of server keys (rxrpc_s type) to the appropriate
security class.
(5) Organise the security bits in the rxrpc_connection struct into a
union to make it easier to override for other classes.
(6) Move some bits from core code into rxkad that won't be appropriate to
rxgk.
The patches are tagged here:
git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
rxrpc-next-20201123
and can also be found on the following branch:
http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=rxrpc-next
David
---
David Howells (17):
keys: Provide the original description to the key preparser
rxrpc: Remove the rxk5 security class as it's now defunct
rxrpc: List the held token types in the key description in /proc/keys
rxrpc: Support keys with multiple authentication tokens
rxrpc: Don't retain the server key in the connection
rxrpc: Split the server key type (rxrpc_s) into its own file
rxrpc: Hand server key parsing off to the security class
rxrpc: Don't leak the service-side session key to userspace
rxrpc: Allow security classes to give more info on server keys
rxrpc: Make the parsing of xdr payloads more coherent
rxrpc: Ignore unknown tokens in key payload unless no known tokens
rxrpc: Fix example key name in a comment
rxrpc: Merge prime_packet_security into init_connection_security
rxrpc: Don't reserve security header in Tx DATA skbuff
rxrpc: Organise connection security to use a union
rxrpc: rxkad: Don't use pskb_pull() to advance through the response packet
rxrpc: Ask the security class how much space to allow in a packet
include/keys/rxrpc-type.h | 56 +---
net/rxrpc/Makefile | 1 +
net/rxrpc/ar-internal.h | 63 ++--
net/rxrpc/call_accept.c | 14 +-
net/rxrpc/conn_client.c | 6 -
net/rxrpc/conn_event.c | 8 +-
net/rxrpc/conn_object.c | 2 -
net/rxrpc/conn_service.c | 2 -
net/rxrpc/insecure.c | 19 +-
net/rxrpc/key.c | 658 ++++----------------------------------
net/rxrpc/rxkad.c | 256 ++++++++++-----
net/rxrpc/security.c | 98 ++++--
net/rxrpc/sendmsg.c | 45 +--
net/rxrpc/server_key.c | 143 +++++++++
14 files changed, 519 insertions(+), 852 deletions(-)
create mode 100644 net/rxrpc/server_key.c
^ permalink raw reply
* [PATCH net-next 02/17] rxrpc: Remove the rxk5 security class as it's now defunct
From: David Howells @ 2020-11-23 20:10 UTC (permalink / raw)
To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <160616220405.830164.2239716599743995145.stgit@warthog.procyon.org.uk>
Remove the rxrpc rxk5 security class as it's now defunct and nothing uses
it anymore.
Signed-off-by: David Howells <dhowells@redhat.com>
---
include/keys/rxrpc-type.h | 55 -----
net/rxrpc/key.c | 468 ---------------------------------------------
2 files changed, 523 deletions(-)
diff --git a/include/keys/rxrpc-type.h b/include/keys/rxrpc-type.h
index 2b0b15a71228..8e4ced9b4ecf 100644
--- a/include/keys/rxrpc-type.h
+++ b/include/keys/rxrpc-type.h
@@ -31,54 +31,6 @@ struct rxkad_key {
u8 ticket[]; /* the encrypted ticket */
};
-/*
- * Kerberos 5 principal
- * name/name/name@realm
- */
-struct krb5_principal {
- u8 n_name_parts; /* N of parts of the name part of the principal */
- char **name_parts; /* parts of the name part of the principal */
- char *realm; /* parts of the realm part of the principal */
-};
-
-/*
- * Kerberos 5 tagged data
- */
-struct krb5_tagged_data {
- /* for tag value, see /usr/include/krb5/krb5.h
- * - KRB5_AUTHDATA_* for auth data
- * -
- */
- s32 tag;
- u32 data_len;
- u8 *data;
-};
-
-/*
- * RxRPC key for Kerberos V (type-5 security)
- */
-struct rxk5_key {
- u64 authtime; /* time at which auth token generated */
- u64 starttime; /* time at which auth token starts */
- u64 endtime; /* time at which auth token expired */
- u64 renew_till; /* time to which auth token can be renewed */
- s32 is_skey; /* T if ticket is encrypted in another ticket's
- * skey */
- s32 flags; /* mask of TKT_FLG_* bits (krb5/krb5.h) */
- struct krb5_principal client; /* client principal name */
- struct krb5_principal server; /* server principal name */
- u16 ticket_len; /* length of ticket */
- u16 ticket2_len; /* length of second ticket */
- u8 n_authdata; /* number of authorisation data elements */
- u8 n_addresses; /* number of addresses */
- struct krb5_tagged_data session; /* session data; tag is enctype */
- struct krb5_tagged_data *addresses; /* addresses */
- u8 *ticket; /* krb5 ticket */
- u8 *ticket2; /* second krb5 ticket, if related to ticket (via
- * DUPLICATE-SKEY or ENC-TKT-IN-SKEY) */
- struct krb5_tagged_data *authdata; /* authorisation data */
-};
-
/*
* list of tokens attached to an rxrpc key
*/
@@ -87,7 +39,6 @@ struct rxrpc_key_token {
struct rxrpc_key_token *next; /* the next token in the list */
union {
struct rxkad_key *kad;
- struct rxk5_key *k5;
};
};
@@ -116,12 +67,6 @@ struct rxrpc_key_data_v1 {
#define AFSTOKEN_RK_TIX_MAX 12000 /* max RxKAD ticket size */
#define AFSTOKEN_GK_KEY_MAX 64 /* max GSSAPI key size */
#define AFSTOKEN_GK_TOKEN_MAX 16384 /* max GSSAPI token size */
-#define AFSTOKEN_K5_COMPONENTS_MAX 16 /* max K5 components */
-#define AFSTOKEN_K5_NAME_MAX 128 /* max K5 name length */
-#define AFSTOKEN_K5_REALM_MAX 64 /* max K5 realm name length */
-#define AFSTOKEN_K5_TIX_MAX 16384 /* max K5 ticket size */
-#define AFSTOKEN_K5_ADDRESSES_MAX 16 /* max K5 addresses */
-#define AFSTOKEN_K5_AUTHDATA_MAX 16 /* max K5 pieces of auth data */
/*
* Truncate a time64_t to the range from 1970 to 2106 as in the network
diff --git a/net/rxrpc/key.c b/net/rxrpc/key.c
index 2e8bd3b97301..fb4d2a2fca02 100644
--- a/net/rxrpc/key.c
+++ b/net/rxrpc/key.c
@@ -165,391 +165,6 @@ static int rxrpc_preparse_xdr_rxkad(struct key_preparsed_payload *prep,
return 0;
}
-static void rxrpc_free_krb5_principal(struct krb5_principal *princ)
-{
- int loop;
-
- if (princ->name_parts) {
- for (loop = princ->n_name_parts - 1; loop >= 0; loop--)
- kfree(princ->name_parts[loop]);
- kfree(princ->name_parts);
- }
- kfree(princ->realm);
-}
-
-static void rxrpc_free_krb5_tagged(struct krb5_tagged_data *td)
-{
- kfree(td->data);
-}
-
-/*
- * free up an RxK5 token
- */
-static void rxrpc_rxk5_free(struct rxk5_key *rxk5)
-{
- int loop;
-
- rxrpc_free_krb5_principal(&rxk5->client);
- rxrpc_free_krb5_principal(&rxk5->server);
- rxrpc_free_krb5_tagged(&rxk5->session);
-
- if (rxk5->addresses) {
- for (loop = rxk5->n_addresses - 1; loop >= 0; loop--)
- rxrpc_free_krb5_tagged(&rxk5->addresses[loop]);
- kfree(rxk5->addresses);
- }
- if (rxk5->authdata) {
- for (loop = rxk5->n_authdata - 1; loop >= 0; loop--)
- rxrpc_free_krb5_tagged(&rxk5->authdata[loop]);
- kfree(rxk5->authdata);
- }
-
- kfree(rxk5->ticket);
- kfree(rxk5->ticket2);
- kfree(rxk5);
-}
-
-/*
- * extract a krb5 principal
- */
-static int rxrpc_krb5_decode_principal(struct krb5_principal *princ,
- const __be32 **_xdr,
- unsigned int *_toklen)
-{
- const __be32 *xdr = *_xdr;
- unsigned int toklen = *_toklen, n_parts, loop, tmp, paddedlen;
-
- /* there must be at least one name, and at least #names+1 length
- * words */
- if (toklen <= 12)
- return -EINVAL;
-
- _enter(",{%x,%x,%x},%u",
- ntohl(xdr[0]), ntohl(xdr[1]), ntohl(xdr[2]), toklen);
-
- n_parts = ntohl(*xdr++);
- toklen -= 4;
- if (n_parts <= 0 || n_parts > AFSTOKEN_K5_COMPONENTS_MAX)
- return -EINVAL;
- princ->n_name_parts = n_parts;
-
- if (toklen <= (n_parts + 1) * 4)
- return -EINVAL;
-
- princ->name_parts = kcalloc(n_parts, sizeof(char *), GFP_KERNEL);
- if (!princ->name_parts)
- return -ENOMEM;
-
- for (loop = 0; loop < n_parts; loop++) {
- if (toklen < 4)
- return -EINVAL;
- tmp = ntohl(*xdr++);
- toklen -= 4;
- if (tmp <= 0 || tmp > AFSTOKEN_STRING_MAX)
- return -EINVAL;
- paddedlen = (tmp + 3) & ~3;
- if (paddedlen > toklen)
- return -EINVAL;
- princ->name_parts[loop] = kmalloc(tmp + 1, GFP_KERNEL);
- if (!princ->name_parts[loop])
- return -ENOMEM;
- memcpy(princ->name_parts[loop], xdr, tmp);
- princ->name_parts[loop][tmp] = 0;
- toklen -= paddedlen;
- xdr += paddedlen >> 2;
- }
-
- if (toklen < 4)
- return -EINVAL;
- tmp = ntohl(*xdr++);
- toklen -= 4;
- if (tmp <= 0 || tmp > AFSTOKEN_K5_REALM_MAX)
- return -EINVAL;
- paddedlen = (tmp + 3) & ~3;
- if (paddedlen > toklen)
- return -EINVAL;
- princ->realm = kmalloc(tmp + 1, GFP_KERNEL);
- if (!princ->realm)
- return -ENOMEM;
- memcpy(princ->realm, xdr, tmp);
- princ->realm[tmp] = 0;
- toklen -= paddedlen;
- xdr += paddedlen >> 2;
-
- _debug("%s/...@%s", princ->name_parts[0], princ->realm);
-
- *_xdr = xdr;
- *_toklen = toklen;
- _leave(" = 0 [toklen=%u]", toklen);
- return 0;
-}
-
-/*
- * extract a piece of krb5 tagged data
- */
-static int rxrpc_krb5_decode_tagged_data(struct krb5_tagged_data *td,
- size_t max_data_size,
- const __be32 **_xdr,
- unsigned int *_toklen)
-{
- const __be32 *xdr = *_xdr;
- unsigned int toklen = *_toklen, len, paddedlen;
-
- /* there must be at least one tag and one length word */
- if (toklen <= 8)
- return -EINVAL;
-
- _enter(",%zu,{%x,%x},%u",
- max_data_size, ntohl(xdr[0]), ntohl(xdr[1]), toklen);
-
- td->tag = ntohl(*xdr++);
- len = ntohl(*xdr++);
- toklen -= 8;
- if (len > max_data_size)
- return -EINVAL;
- paddedlen = (len + 3) & ~3;
- if (paddedlen > toklen)
- return -EINVAL;
- td->data_len = len;
-
- if (len > 0) {
- td->data = kmemdup(xdr, len, GFP_KERNEL);
- if (!td->data)
- return -ENOMEM;
- toklen -= paddedlen;
- xdr += paddedlen >> 2;
- }
-
- _debug("tag %x len %x", td->tag, td->data_len);
-
- *_xdr = xdr;
- *_toklen = toklen;
- _leave(" = 0 [toklen=%u]", toklen);
- return 0;
-}
-
-/*
- * extract an array of tagged data
- */
-static int rxrpc_krb5_decode_tagged_array(struct krb5_tagged_data **_td,
- u8 *_n_elem,
- u8 max_n_elem,
- size_t max_elem_size,
- const __be32 **_xdr,
- unsigned int *_toklen)
-{
- struct krb5_tagged_data *td;
- const __be32 *xdr = *_xdr;
- unsigned int toklen = *_toklen, n_elem, loop;
- int ret;
-
- /* there must be at least one count */
- if (toklen < 4)
- return -EINVAL;
-
- _enter(",,%u,%zu,{%x},%u",
- max_n_elem, max_elem_size, ntohl(xdr[0]), toklen);
-
- n_elem = ntohl(*xdr++);
- toklen -= 4;
- if (n_elem > max_n_elem)
- return -EINVAL;
- *_n_elem = n_elem;
- if (n_elem > 0) {
- if (toklen <= (n_elem + 1) * 4)
- return -EINVAL;
-
- _debug("n_elem %d", n_elem);
-
- td = kcalloc(n_elem, sizeof(struct krb5_tagged_data),
- GFP_KERNEL);
- if (!td)
- return -ENOMEM;
- *_td = td;
-
- for (loop = 0; loop < n_elem; loop++) {
- ret = rxrpc_krb5_decode_tagged_data(&td[loop],
- max_elem_size,
- &xdr, &toklen);
- if (ret < 0)
- return ret;
- }
- }
-
- *_xdr = xdr;
- *_toklen = toklen;
- _leave(" = 0 [toklen=%u]", toklen);
- return 0;
-}
-
-/*
- * extract a krb5 ticket
- */
-static int rxrpc_krb5_decode_ticket(u8 **_ticket, u16 *_tktlen,
- const __be32 **_xdr, unsigned int *_toklen)
-{
- const __be32 *xdr = *_xdr;
- unsigned int toklen = *_toklen, len, paddedlen;
-
- /* there must be at least one length word */
- if (toklen <= 4)
- return -EINVAL;
-
- _enter(",{%x},%u", ntohl(xdr[0]), toklen);
-
- len = ntohl(*xdr++);
- toklen -= 4;
- if (len > AFSTOKEN_K5_TIX_MAX)
- return -EINVAL;
- paddedlen = (len + 3) & ~3;
- if (paddedlen > toklen)
- return -EINVAL;
- *_tktlen = len;
-
- _debug("ticket len %u", len);
-
- if (len > 0) {
- *_ticket = kmemdup(xdr, len, GFP_KERNEL);
- if (!*_ticket)
- return -ENOMEM;
- toklen -= paddedlen;
- xdr += paddedlen >> 2;
- }
-
- *_xdr = xdr;
- *_toklen = toklen;
- _leave(" = 0 [toklen=%u]", toklen);
- return 0;
-}
-
-/*
- * parse an RxK5 type XDR format token
- * - the caller guarantees we have at least 4 words
- */
-static int rxrpc_preparse_xdr_rxk5(struct key_preparsed_payload *prep,
- size_t datalen,
- const __be32 *xdr, unsigned int toklen)
-{
- struct rxrpc_key_token *token, **pptoken;
- struct rxk5_key *rxk5;
- const __be32 *end_xdr = xdr + (toklen >> 2);
- time64_t expiry;
- int ret;
-
- _enter(",{%x,%x,%x,%x},%u",
- ntohl(xdr[0]), ntohl(xdr[1]), ntohl(xdr[2]), ntohl(xdr[3]),
- toklen);
-
- /* reserve some payload space for this subkey - the length of the token
- * is a reasonable approximation */
- prep->quotalen = datalen + toklen;
-
- token = kzalloc(sizeof(*token), GFP_KERNEL);
- if (!token)
- return -ENOMEM;
-
- rxk5 = kzalloc(sizeof(*rxk5), GFP_KERNEL);
- if (!rxk5) {
- kfree(token);
- return -ENOMEM;
- }
-
- token->security_index = RXRPC_SECURITY_RXK5;
- token->k5 = rxk5;
-
- /* extract the principals */
- ret = rxrpc_krb5_decode_principal(&rxk5->client, &xdr, &toklen);
- if (ret < 0)
- goto error;
- ret = rxrpc_krb5_decode_principal(&rxk5->server, &xdr, &toklen);
- if (ret < 0)
- goto error;
-
- /* extract the session key and the encoding type (the tag field ->
- * ENCTYPE_xxx) */
- ret = rxrpc_krb5_decode_tagged_data(&rxk5->session, AFSTOKEN_DATA_MAX,
- &xdr, &toklen);
- if (ret < 0)
- goto error;
-
- if (toklen < 4 * 8 + 2 * 4)
- goto inval;
- rxk5->authtime = be64_to_cpup((const __be64 *) xdr);
- xdr += 2;
- rxk5->starttime = be64_to_cpup((const __be64 *) xdr);
- xdr += 2;
- rxk5->endtime = be64_to_cpup((const __be64 *) xdr);
- xdr += 2;
- rxk5->renew_till = be64_to_cpup((const __be64 *) xdr);
- xdr += 2;
- rxk5->is_skey = ntohl(*xdr++);
- rxk5->flags = ntohl(*xdr++);
- toklen -= 4 * 8 + 2 * 4;
-
- _debug("times: a=%llx s=%llx e=%llx rt=%llx",
- rxk5->authtime, rxk5->starttime, rxk5->endtime,
- rxk5->renew_till);
- _debug("is_skey=%x flags=%x", rxk5->is_skey, rxk5->flags);
-
- /* extract the permitted client addresses */
- ret = rxrpc_krb5_decode_tagged_array(&rxk5->addresses,
- &rxk5->n_addresses,
- AFSTOKEN_K5_ADDRESSES_MAX,
- AFSTOKEN_DATA_MAX,
- &xdr, &toklen);
- if (ret < 0)
- goto error;
-
- ASSERTCMP((end_xdr - xdr) << 2, ==, toklen);
-
- /* extract the tickets */
- ret = rxrpc_krb5_decode_ticket(&rxk5->ticket, &rxk5->ticket_len,
- &xdr, &toklen);
- if (ret < 0)
- goto error;
- ret = rxrpc_krb5_decode_ticket(&rxk5->ticket2, &rxk5->ticket2_len,
- &xdr, &toklen);
- if (ret < 0)
- goto error;
-
- ASSERTCMP((end_xdr - xdr) << 2, ==, toklen);
-
- /* extract the typed auth data */
- ret = rxrpc_krb5_decode_tagged_array(&rxk5->authdata,
- &rxk5->n_authdata,
- AFSTOKEN_K5_AUTHDATA_MAX,
- AFSTOKEN_BDATALN_MAX,
- &xdr, &toklen);
- if (ret < 0)
- goto error;
-
- ASSERTCMP((end_xdr - xdr) << 2, ==, toklen);
-
- if (toklen != 0)
- goto inval;
-
- /* attach the payload */
- for (pptoken = (struct rxrpc_key_token **)&prep->payload.data[0];
- *pptoken;
- pptoken = &(*pptoken)->next)
- continue;
- *pptoken = token;
- expiry = rxrpc_u32_to_time64(token->k5->endtime);
- if (expiry < prep->expiry)
- prep->expiry = expiry;
-
- _leave(" = 0");
- return 0;
-
-inval:
- ret = -EINVAL;
-error:
- rxrpc_rxk5_free(rxk5);
- kfree(token);
- _leave(" = %d", ret);
- return ret;
-}
-
/*
* attempt to parse the data as the XDR format
* - the caller guarantees we have more than 7 words
@@ -650,12 +265,6 @@ static int rxrpc_preparse_xdr(struct key_preparsed_payload *prep)
goto error;
break;
- case RXRPC_SECURITY_RXK5:
- ret = rxrpc_preparse_xdr_rxk5(prep, datalen, xdr, toklen);
- if (ret != 0)
- goto error;
- break;
-
default:
ret = -EPROTONOSUPPORT;
goto error;
@@ -805,10 +414,6 @@ static void rxrpc_free_token_list(struct rxrpc_key_token *token)
case RXRPC_SECURITY_RXKAD:
kfree(token->kad);
break;
- case RXRPC_SECURITY_RXK5:
- if (token->k5)
- rxrpc_rxk5_free(token->k5);
- break;
default:
pr_err("Unknown token type %x on rxrpc key\n",
token->security_index);
@@ -1044,12 +649,10 @@ static long rxrpc_read(const struct key *key,
char *buffer, size_t buflen)
{
const struct rxrpc_key_token *token;
- const struct krb5_principal *princ;
size_t size;
__be32 *xdr, *oldxdr;
u32 cnlen, toksize, ntoks, tok, zero;
u16 toksizes[AFSTOKEN_MAX];
- int loop;
_enter("");
@@ -1077,35 +680,6 @@ static long rxrpc_read(const struct key *key,
toksize += RND(token->kad->ticket_len);
break;
- case RXRPC_SECURITY_RXK5:
- princ = &token->k5->client;
- toksize += 4 + princ->n_name_parts * 4;
- for (loop = 0; loop < princ->n_name_parts; loop++)
- toksize += RND(strlen(princ->name_parts[loop]));
- toksize += 4 + RND(strlen(princ->realm));
-
- princ = &token->k5->server;
- toksize += 4 + princ->n_name_parts * 4;
- for (loop = 0; loop < princ->n_name_parts; loop++)
- toksize += RND(strlen(princ->name_parts[loop]));
- toksize += 4 + RND(strlen(princ->realm));
-
- toksize += 8 + RND(token->k5->session.data_len);
-
- toksize += 4 * 8 + 2 * 4;
-
- toksize += 4 + token->k5->n_addresses * 8;
- for (loop = 0; loop < token->k5->n_addresses; loop++)
- toksize += RND(token->k5->addresses[loop].data_len);
-
- toksize += 4 + RND(token->k5->ticket_len);
- toksize += 4 + RND(token->k5->ticket2_len);
-
- toksize += 4 + token->k5->n_authdata * 8;
- for (loop = 0; loop < token->k5->n_authdata; loop++)
- toksize += RND(token->k5->authdata[loop].data_len);
- break;
-
default: /* we have a ticket we can't encode */
pr_err("Unsupported key token type (%u)\n",
token->security_index);
@@ -1181,48 +755,6 @@ static long rxrpc_read(const struct key *key,
ENCODE_DATA(token->kad->ticket_len, token->kad->ticket);
break;
- case RXRPC_SECURITY_RXK5:
- princ = &token->k5->client;
- ENCODE(princ->n_name_parts);
- for (loop = 0; loop < princ->n_name_parts; loop++)
- ENCODE_STR(princ->name_parts[loop]);
- ENCODE_STR(princ->realm);
-
- princ = &token->k5->server;
- ENCODE(princ->n_name_parts);
- for (loop = 0; loop < princ->n_name_parts; loop++)
- ENCODE_STR(princ->name_parts[loop]);
- ENCODE_STR(princ->realm);
-
- ENCODE(token->k5->session.tag);
- ENCODE_DATA(token->k5->session.data_len,
- token->k5->session.data);
-
- ENCODE64(token->k5->authtime);
- ENCODE64(token->k5->starttime);
- ENCODE64(token->k5->endtime);
- ENCODE64(token->k5->renew_till);
- ENCODE(token->k5->is_skey);
- ENCODE(token->k5->flags);
-
- ENCODE(token->k5->n_addresses);
- for (loop = 0; loop < token->k5->n_addresses; loop++) {
- ENCODE(token->k5->addresses[loop].tag);
- ENCODE_DATA(token->k5->addresses[loop].data_len,
- token->k5->addresses[loop].data);
- }
-
- ENCODE_DATA(token->k5->ticket_len, token->k5->ticket);
- ENCODE_DATA(token->k5->ticket2_len, token->k5->ticket2);
-
- ENCODE(token->k5->n_authdata);
- for (loop = 0; loop < token->k5->n_authdata; loop++) {
- ENCODE(token->k5->authdata[loop].tag);
- ENCODE_DATA(token->k5->authdata[loop].data_len,
- token->k5->authdata[loop].data);
- }
- break;
-
default:
break;
}
^ permalink raw reply related
* [PATCH net-next 04/17] rxrpc: Support keys with multiple authentication tokens
From: David Howells @ 2020-11-23 20:10 UTC (permalink / raw)
To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <160616220405.830164.2239716599743995145.stgit@warthog.procyon.org.uk>
rxrpc-type keys can have multiple tokens attached for different security
classes. Currently, rxrpc always picks the first one, whether or not the
security class it indicates is supported.
Add preliminary support for choosing which security class will be used
(this will need to be directed from a higher layer) and go through the
tokens to find one that's supported.
Signed-off-by: David Howells <dhowells@redhat.com>
---
net/rxrpc/ar-internal.h | 4 +++-
net/rxrpc/conn_event.c | 3 ++-
net/rxrpc/insecure.c | 3 ++-
net/rxrpc/rxkad.c | 5 ++---
net/rxrpc/security.c | 15 ++++++++-------
5 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index dce48162f6c2..3c417ec94e4c 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -12,6 +12,7 @@
#include <net/netns/generic.h>
#include <net/sock.h>
#include <net/af_rxrpc.h>
+#include <keys/rxrpc-type.h>
#include "protocol.h"
#if 0
@@ -217,7 +218,8 @@ struct rxrpc_security {
void (*exit)(void);
/* initialise a connection's security */
- int (*init_connection_security)(struct rxrpc_connection *);
+ int (*init_connection_security)(struct rxrpc_connection *,
+ struct rxrpc_key_token *);
/* prime a connection's packet security */
int (*prime_packet_security)(struct rxrpc_connection *);
diff --git a/net/rxrpc/conn_event.c b/net/rxrpc/conn_event.c
index aff184145ffa..bbf86203ed25 100644
--- a/net/rxrpc/conn_event.c
+++ b/net/rxrpc/conn_event.c
@@ -333,7 +333,8 @@ static int rxrpc_process_event(struct rxrpc_connection *conn,
if (ret < 0)
return ret;
- ret = conn->security->init_connection_security(conn);
+ ret = conn->security->init_connection_security(
+ conn, conn->params.key->payload.data[0]);
if (ret < 0)
return ret;
diff --git a/net/rxrpc/insecure.c b/net/rxrpc/insecure.c
index f6c59f5fae9d..cf3ecffcf424 100644
--- a/net/rxrpc/insecure.c
+++ b/net/rxrpc/insecure.c
@@ -8,7 +8,8 @@
#include <net/af_rxrpc.h>
#include "ar-internal.h"
-static int none_init_connection_security(struct rxrpc_connection *conn)
+static int none_init_connection_security(struct rxrpc_connection *conn,
+ struct rxrpc_key_token *token)
{
return 0;
}
diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c
index f114dc2af5cf..404d1323c239 100644
--- a/net/rxrpc/rxkad.c
+++ b/net/rxrpc/rxkad.c
@@ -49,15 +49,14 @@ static DEFINE_MUTEX(rxkad_ci_mutex);
/*
* initialise connection security
*/
-static int rxkad_init_connection_security(struct rxrpc_connection *conn)
+static int rxkad_init_connection_security(struct rxrpc_connection *conn,
+ struct rxrpc_key_token *token)
{
struct crypto_sync_skcipher *ci;
- struct rxrpc_key_token *token;
int ret;
_enter("{%d},{%x}", conn->debug_id, key_serial(conn->params.key));
- token = conn->params.key->payload.data[0];
conn->security_ix = token->security_index;
ci = crypto_alloc_sync_skcipher("pcbc(fcrypt)", 0, 0);
diff --git a/net/rxrpc/security.c b/net/rxrpc/security.c
index 9b1fb9ed0717..0c5168f52bd6 100644
--- a/net/rxrpc/security.c
+++ b/net/rxrpc/security.c
@@ -81,16 +81,17 @@ int rxrpc_init_client_conn_security(struct rxrpc_connection *conn)
if (ret < 0)
return ret;
- token = key->payload.data[0];
- if (!token)
- return -EKEYREJECTED;
+ for (token = key->payload.data[0]; token; token = token->next) {
+ sec = rxrpc_security_lookup(token->security_index);
+ if (sec)
+ goto found;
+ }
+ return -EKEYREJECTED;
- sec = rxrpc_security_lookup(token->security_index);
- if (!sec)
- return -EKEYREJECTED;
+found:
conn->security = sec;
- ret = conn->security->init_connection_security(conn);
+ ret = conn->security->init_connection_security(conn, token);
if (ret < 0) {
conn->security = &rxrpc_no_security;
return ret;
^ permalink raw reply related
* [PATCH net-next 05/17] rxrpc: Don't retain the server key in the connection
From: David Howells @ 2020-11-23 20:10 UTC (permalink / raw)
To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <160616220405.830164.2239716599743995145.stgit@warthog.procyon.org.uk>
Don't retain a pointer to the server key in the connection, but rather get
it on demand when the server has to deal with a response packet.
This is necessary to implement RxGK (GSSAPI-mediated transport class),
where we can't know which key we'll need until we've challenged the client
and got back the response.
This also means that we don't need to do a key search in the accept path in
softirq mode.
Also, whilst we're at it, allow the security class to ask for a kvno and
encoding-type variant of a server key as RxGK needs different keys for
different encoding types. Keys of this type have an extra bit in the
description:
"<service-id>:<security-index>:<kvno>:<enctype>"
Signed-off-by: David Howells <dhowells@redhat.com>
---
net/rxrpc/ar-internal.h | 11 +++---
net/rxrpc/call_accept.c | 14 ++++----
net/rxrpc/conn_event.c | 1 -
net/rxrpc/conn_object.c | 1 -
net/rxrpc/conn_service.c | 2 -
net/rxrpc/rxkad.c | 57 ++++++++++++++++++--------------
net/rxrpc/security.c | 81 ++++++++++++++++++++++++++++++++--------------
7 files changed, 100 insertions(+), 67 deletions(-)
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 3c417ec94e4c..db6e754743fb 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -441,7 +441,6 @@ struct rxrpc_connection {
struct list_head link; /* link in master connection list */
struct sk_buff_head rx_queue; /* received conn-level packets */
const struct rxrpc_security *security; /* applied security module */
- struct key *server_key; /* security for this service */
struct crypto_sync_skcipher *cipher; /* encryption handle */
struct rxrpc_crypt csum_iv; /* packet checksum base */
unsigned long flags;
@@ -890,8 +889,7 @@ struct rxrpc_connection *rxrpc_find_service_conn_rcu(struct rxrpc_peer *,
struct sk_buff *);
struct rxrpc_connection *rxrpc_prealloc_service_connection(struct rxrpc_net *, gfp_t);
void rxrpc_new_incoming_connection(struct rxrpc_sock *, struct rxrpc_connection *,
- const struct rxrpc_security *, struct key *,
- struct sk_buff *);
+ const struct rxrpc_security *, struct sk_buff *);
void rxrpc_unpublish_service_conn(struct rxrpc_connection *);
/*
@@ -1056,9 +1054,10 @@ extern const struct rxrpc_security rxkad;
int __init rxrpc_init_security(void);
void rxrpc_exit_security(void);
int rxrpc_init_client_conn_security(struct rxrpc_connection *);
-bool rxrpc_look_up_server_security(struct rxrpc_local *, struct rxrpc_sock *,
- const struct rxrpc_security **, struct key **,
- struct sk_buff *);
+const struct rxrpc_security *rxrpc_get_incoming_security(struct rxrpc_sock *,
+ struct sk_buff *);
+struct key *rxrpc_look_up_server_security(struct rxrpc_connection *,
+ struct sk_buff *, u32, u32);
/*
* sendmsg.c
diff --git a/net/rxrpc/call_accept.c b/net/rxrpc/call_accept.c
index 8df1964db333..382add72c66f 100644
--- a/net/rxrpc/call_accept.c
+++ b/net/rxrpc/call_accept.c
@@ -261,7 +261,6 @@ static struct rxrpc_call *rxrpc_alloc_incoming_call(struct rxrpc_sock *rx,
struct rxrpc_peer *peer,
struct rxrpc_connection *conn,
const struct rxrpc_security *sec,
- struct key *key,
struct sk_buff *skb)
{
struct rxrpc_backlog *b = rx->backlog;
@@ -309,7 +308,7 @@ static struct rxrpc_call *rxrpc_alloc_incoming_call(struct rxrpc_sock *rx,
conn->params.local = rxrpc_get_local(local);
conn->params.peer = peer;
rxrpc_see_connection(conn);
- rxrpc_new_incoming_connection(rx, conn, sec, key, skb);
+ rxrpc_new_incoming_connection(rx, conn, sec, skb);
} else {
rxrpc_get_connection(conn);
}
@@ -353,7 +352,6 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
struct rxrpc_connection *conn;
struct rxrpc_peer *peer = NULL;
struct rxrpc_call *call = NULL;
- struct key *key = NULL;
_enter("");
@@ -374,11 +372,13 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
*/
conn = rxrpc_find_connection_rcu(local, skb, &peer);
- if (!conn && !rxrpc_look_up_server_security(local, rx, &sec, &key, skb))
- goto no_call;
+ if (!conn) {
+ sec = rxrpc_get_incoming_security(rx, skb);
+ if (!sec)
+ goto no_call;
+ }
- call = rxrpc_alloc_incoming_call(rx, local, peer, conn, sec, key, skb);
- key_put(key);
+ call = rxrpc_alloc_incoming_call(rx, local, peer, conn, sec, skb);
if (!call) {
skb->mark = RXRPC_SKB_MARK_REJECT_BUSY;
goto no_call;
diff --git a/net/rxrpc/conn_event.c b/net/rxrpc/conn_event.c
index bbf86203ed25..03a482ba770f 100644
--- a/net/rxrpc/conn_event.c
+++ b/net/rxrpc/conn_event.c
@@ -378,7 +378,6 @@ static void rxrpc_secure_connection(struct rxrpc_connection *conn)
_enter("{%d}", conn->debug_id);
ASSERT(conn->security_ix != 0);
- ASSERT(conn->server_key);
if (conn->security->issue_challenge(conn) < 0) {
abort_code = RX_CALL_DEAD;
diff --git a/net/rxrpc/conn_object.c b/net/rxrpc/conn_object.c
index 3bcbe0665f91..8dd1ef25b98f 100644
--- a/net/rxrpc/conn_object.c
+++ b/net/rxrpc/conn_object.c
@@ -363,7 +363,6 @@ static void rxrpc_destroy_connection(struct rcu_head *rcu)
conn->security->clear(conn);
key_put(conn->params.key);
- key_put(conn->server_key);
rxrpc_put_bundle(conn->bundle);
rxrpc_put_peer(conn->params.peer);
diff --git a/net/rxrpc/conn_service.c b/net/rxrpc/conn_service.c
index 6c847720494f..e1966dfc9152 100644
--- a/net/rxrpc/conn_service.c
+++ b/net/rxrpc/conn_service.c
@@ -156,7 +156,6 @@ struct rxrpc_connection *rxrpc_prealloc_service_connection(struct rxrpc_net *rxn
void rxrpc_new_incoming_connection(struct rxrpc_sock *rx,
struct rxrpc_connection *conn,
const struct rxrpc_security *sec,
- struct key *key,
struct sk_buff *skb)
{
struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
@@ -170,7 +169,6 @@ void rxrpc_new_incoming_connection(struct rxrpc_sock *rx,
conn->security_ix = sp->hdr.securityIndex;
conn->out_clientflag = 0;
conn->security = sec;
- conn->server_key = key_get(key);
if (conn->security_ix)
conn->state = RXRPC_CONN_SERVICE_UNSECURED;
else
diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c
index 404d1323c239..0d21935dac27 100644
--- a/net/rxrpc/rxkad.c
+++ b/net/rxrpc/rxkad.c
@@ -647,11 +647,7 @@ static int rxkad_issue_challenge(struct rxrpc_connection *conn)
u32 serial;
int ret;
- _enter("{%d,%x}", conn->debug_id, key_serial(conn->server_key));
-
- ret = key_validate(conn->server_key);
- if (ret < 0)
- return ret;
+ _enter("{%d}", conn->debug_id);
get_random_bytes(&conn->security_nonce, sizeof(conn->security_nonce));
@@ -891,6 +887,7 @@ static int rxkad_respond_to_challenge(struct rxrpc_connection *conn,
* decrypt the kerberos IV ticket in the response
*/
static int rxkad_decrypt_ticket(struct rxrpc_connection *conn,
+ struct key *server_key,
struct sk_buff *skb,
void *ticket, size_t ticket_len,
struct rxrpc_crypt *_session_key,
@@ -910,30 +907,17 @@ static int rxkad_decrypt_ticket(struct rxrpc_connection *conn,
u32 abort_code;
u8 *p, *q, *name, *end;
- _enter("{%d},{%x}", conn->debug_id, key_serial(conn->server_key));
+ _enter("{%d},{%x}", conn->debug_id, key_serial(server_key));
*_expiry = 0;
- ret = key_validate(conn->server_key);
- if (ret < 0) {
- switch (ret) {
- case -EKEYEXPIRED:
- abort_code = RXKADEXPIRED;
- goto other_error;
- default:
- abort_code = RXKADNOAUTH;
- goto other_error;
- }
- }
-
- ASSERT(conn->server_key->payload.data[0] != NULL);
+ ASSERT(server_key->payload.data[0] != NULL);
ASSERTCMP((unsigned long) ticket & 7UL, ==, 0);
- memcpy(&iv, &conn->server_key->payload.data[2], sizeof(iv));
+ memcpy(&iv, &server_key->payload.data[2], sizeof(iv));
ret = -ENOMEM;
- req = skcipher_request_alloc(conn->server_key->payload.data[0],
- GFP_NOFS);
+ req = skcipher_request_alloc(server_key->payload.data[0], GFP_NOFS);
if (!req)
goto temporary_error;
@@ -1089,6 +1073,7 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
struct rxkad_response *response;
struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
struct rxrpc_crypt session_key;
+ struct key *server_key;
const char *eproto;
time64_t expiry;
void *ticket;
@@ -1096,7 +1081,27 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
__be32 csum;
int ret, i;
- _enter("{%d,%x}", conn->debug_id, key_serial(conn->server_key));
+ _enter("{%d}", conn->debug_id);
+
+ server_key = rxrpc_look_up_server_security(conn, skb, 0, 0);
+ if (IS_ERR(server_key)) {
+ switch (PTR_ERR(server_key)) {
+ case -ENOKEY:
+ abort_code = RXKADUNKNOWNKEY;
+ break;
+ case -EKEYEXPIRED:
+ abort_code = RXKADEXPIRED;
+ break;
+ default:
+ abort_code = RXKADNOAUTH;
+ break;
+ }
+ trace_rxrpc_abort(0, "SVK",
+ sp->hdr.cid, sp->hdr.callNumber, sp->hdr.seq,
+ abort_code, PTR_ERR(server_key));
+ *_abort_code = abort_code;
+ return -EPROTO;
+ }
ret = -ENOMEM;
response = kzalloc(sizeof(struct rxkad_response), GFP_NOFS);
@@ -1144,8 +1149,8 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
ticket, ticket_len) < 0)
goto protocol_error_free;
- ret = rxkad_decrypt_ticket(conn, skb, ticket, ticket_len, &session_key,
- &expiry, _abort_code);
+ ret = rxkad_decrypt_ticket(conn, server_key, skb, ticket, ticket_len,
+ &session_key, &expiry, _abort_code);
if (ret < 0)
goto temporary_error_free_ticket;
@@ -1224,6 +1229,7 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
protocol_error:
kfree(response);
trace_rxrpc_rx_eproto(NULL, sp->hdr.serial, eproto);
+ key_put(server_key);
*_abort_code = abort_code;
return -EPROTO;
@@ -1236,6 +1242,7 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
* ENOMEM. We just want to send the challenge again. Note that we
* also come out this way if the ticket decryption fails.
*/
+ key_put(server_key);
return ret;
}
diff --git a/net/rxrpc/security.c b/net/rxrpc/security.c
index 0c5168f52bd6..bef9971e15cd 100644
--- a/net/rxrpc/security.c
+++ b/net/rxrpc/security.c
@@ -102,22 +102,16 @@ int rxrpc_init_client_conn_security(struct rxrpc_connection *conn)
}
/*
- * Find the security key for a server connection.
+ * Set the ops a server connection.
*/
-bool rxrpc_look_up_server_security(struct rxrpc_local *local, struct rxrpc_sock *rx,
- const struct rxrpc_security **_sec,
- struct key **_key,
- struct sk_buff *skb)
+const struct rxrpc_security *rxrpc_get_incoming_security(struct rxrpc_sock *rx,
+ struct sk_buff *skb)
{
const struct rxrpc_security *sec;
struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
- key_ref_t kref = NULL;
- char kdesc[5 + 1 + 3 + 1];
_enter("");
- sprintf(kdesc, "%u:%u", sp->hdr.serviceId, sp->hdr.securityIndex);
-
sec = rxrpc_security_lookup(sp->hdr.securityIndex);
if (!sec) {
trace_rxrpc_abort(0, "SVS",
@@ -125,35 +119,72 @@ bool rxrpc_look_up_server_security(struct rxrpc_local *local, struct rxrpc_sock
RX_INVALID_OPERATION, EKEYREJECTED);
skb->mark = RXRPC_SKB_MARK_REJECT_ABORT;
skb->priority = RX_INVALID_OPERATION;
- return false;
+ return NULL;
}
- if (sp->hdr.securityIndex == RXRPC_SECURITY_NONE)
- goto out;
-
- if (!rx->securities) {
+ if (sp->hdr.securityIndex != RXRPC_SECURITY_NONE &&
+ !rx->securities) {
trace_rxrpc_abort(0, "SVR",
sp->hdr.cid, sp->hdr.callNumber, sp->hdr.seq,
RX_INVALID_OPERATION, EKEYREJECTED);
skb->mark = RXRPC_SKB_MARK_REJECT_ABORT;
- skb->priority = RX_INVALID_OPERATION;
- return false;
+ skb->priority = sec->no_key_abort;
+ return NULL;
}
+ return sec;
+}
+
+/*
+ * Find the security key for a server connection.
+ */
+struct key *rxrpc_look_up_server_security(struct rxrpc_connection *conn,
+ struct sk_buff *skb,
+ u32 kvno, u32 enctype)
+{
+ struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
+ struct rxrpc_sock *rx;
+ struct key *key = ERR_PTR(-EKEYREJECTED);
+ key_ref_t kref = NULL;
+ char kdesc[5 + 1 + 3 + 1 + 12 + 1 + 12 + 1];
+ int ret;
+
+ _enter("");
+
+ if (enctype)
+ sprintf(kdesc, "%u:%u:%u:%u",
+ sp->hdr.serviceId, sp->hdr.securityIndex, kvno, enctype);
+ else if (kvno)
+ sprintf(kdesc, "%u:%u:%u",
+ sp->hdr.serviceId, sp->hdr.securityIndex, kvno);
+ else
+ sprintf(kdesc, "%u:%u",
+ sp->hdr.serviceId, sp->hdr.securityIndex);
+
+ rcu_read_lock();
+
+ rx = rcu_dereference(conn->params.local->service);
+ if (!rx)
+ goto out;
+
/* look through the service's keyring */
kref = keyring_search(make_key_ref(rx->securities, 1UL),
&key_type_rxrpc_s, kdesc, true);
if (IS_ERR(kref)) {
- trace_rxrpc_abort(0, "SVK",
- sp->hdr.cid, sp->hdr.callNumber, sp->hdr.seq,
- sec->no_key_abort, EKEYREJECTED);
- skb->mark = RXRPC_SKB_MARK_REJECT_ABORT;
- skb->priority = sec->no_key_abort;
- return false;
+ key = ERR_CAST(kref);
+ goto out;
+ }
+
+ key = key_ref_to_ptr(kref);
+
+ ret = key_validate(key);
+ if (ret < 0) {
+ key_put(key);
+ key = ERR_PTR(ret);
+ goto out;
}
out:
- *_sec = sec;
- *_key = key_ref_to_ptr(kref);
- return true;
+ rcu_read_unlock();
+ return key;
}
^ permalink raw reply related
* [PATCH net-next 06/17] rxrpc: Split the server key type (rxrpc_s) into its own file
From: David Howells @ 2020-11-23 20:10 UTC (permalink / raw)
To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <160616220405.830164.2239716599743995145.stgit@warthog.procyon.org.uk>
Split the server private key type (rxrpc_s) out into its own file rather
than mingling it with the authentication/client key type (rxrpc) since they
don't really bear any relation.
Signed-off-by: David Howells <dhowells@redhat.com>
---
net/rxrpc/Makefile | 1
net/rxrpc/ar-internal.h | 9 ++-
net/rxrpc/key.c | 125 ------------------------------------------
net/rxrpc/server_key.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 149 insertions(+), 127 deletions(-)
create mode 100644 net/rxrpc/server_key.c
diff --git a/net/rxrpc/Makefile b/net/rxrpc/Makefile
index ddd0f95713a9..b11281bed2a4 100644
--- a/net/rxrpc/Makefile
+++ b/net/rxrpc/Makefile
@@ -28,6 +28,7 @@ rxrpc-y := \
rtt.o \
security.o \
sendmsg.o \
+ server_key.o \
skbuff.o \
utils.o
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index db6e754743fb..6427bcfb4df5 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -906,10 +906,8 @@ extern const struct rxrpc_security rxrpc_no_security;
* key.c
*/
extern struct key_type key_type_rxrpc;
-extern struct key_type key_type_rxrpc_s;
int rxrpc_request_key(struct rxrpc_sock *, sockptr_t , int);
-int rxrpc_server_keyring(struct rxrpc_sock *, sockptr_t, int);
int rxrpc_get_server_data_key(struct rxrpc_connection *, const void *, time64_t,
u32);
@@ -1064,6 +1062,13 @@ struct key *rxrpc_look_up_server_security(struct rxrpc_connection *,
*/
int rxrpc_do_sendmsg(struct rxrpc_sock *, struct msghdr *, size_t);
+/*
+ * server_key.c
+ */
+extern struct key_type key_type_rxrpc_s;
+
+int rxrpc_server_keyring(struct rxrpc_sock *, sockptr_t, int);
+
/*
* skbuff.c
*/
diff --git a/net/rxrpc/key.c b/net/rxrpc/key.c
index 197b4cf46b64..3bd7b9d48d27 100644
--- a/net/rxrpc/key.c
+++ b/net/rxrpc/key.c
@@ -23,15 +23,10 @@
#include <keys/user-type.h>
#include "ar-internal.h"
-static int rxrpc_vet_description_s(const char *);
static int rxrpc_preparse(struct key_preparsed_payload *);
-static int rxrpc_preparse_s(struct key_preparsed_payload *);
static void rxrpc_free_preparse(struct key_preparsed_payload *);
-static void rxrpc_free_preparse_s(struct key_preparsed_payload *);
static void rxrpc_destroy(struct key *);
-static void rxrpc_destroy_s(struct key *);
static void rxrpc_describe(const struct key *, struct seq_file *);
-static void rxrpc_describe_s(const struct key *, struct seq_file *);
static long rxrpc_read(const struct key *, char *, size_t);
/*
@@ -50,38 +45,6 @@ struct key_type key_type_rxrpc = {
};
EXPORT_SYMBOL(key_type_rxrpc);
-/*
- * rxrpc server defined keys take "<serviceId>:<securityIndex>" as the
- * description and an 8-byte decryption key as the payload
- */
-struct key_type key_type_rxrpc_s = {
- .name = "rxrpc_s",
- .flags = KEY_TYPE_NET_DOMAIN,
- .vet_description = rxrpc_vet_description_s,
- .preparse = rxrpc_preparse_s,
- .free_preparse = rxrpc_free_preparse_s,
- .instantiate = generic_key_instantiate,
- .destroy = rxrpc_destroy_s,
- .describe = rxrpc_describe_s,
-};
-
-/*
- * Vet the description for an RxRPC server key
- */
-static int rxrpc_vet_description_s(const char *desc)
-{
- unsigned long num;
- char *p;
-
- num = simple_strtoul(desc, &p, 10);
- if (*p != ':' || num > 65535)
- return -EINVAL;
- num = simple_strtoul(p + 1, &p, 10);
- if (*p || num < 1 || num > 255)
- return -EINVAL;
- return 0;
-}
-
/*
* parse an RxKAD type XDR format token
* - the caller guarantees we have at least 4 words
@@ -433,45 +396,6 @@ static void rxrpc_free_preparse(struct key_preparsed_payload *prep)
rxrpc_free_token_list(prep->payload.data[0]);
}
-/*
- * Preparse a server secret key.
- *
- * The data should be the 8-byte secret key.
- */
-static int rxrpc_preparse_s(struct key_preparsed_payload *prep)
-{
- struct crypto_skcipher *ci;
-
- _enter("%zu", prep->datalen);
-
- if (prep->datalen != 8)
- return -EINVAL;
-
- memcpy(&prep->payload.data[2], prep->data, 8);
-
- ci = crypto_alloc_skcipher("pcbc(des)", 0, CRYPTO_ALG_ASYNC);
- if (IS_ERR(ci)) {
- _leave(" = %ld", PTR_ERR(ci));
- return PTR_ERR(ci);
- }
-
- if (crypto_skcipher_setkey(ci, prep->data, 8) < 0)
- BUG();
-
- prep->payload.data[0] = ci;
- _leave(" = 0");
- return 0;
-}
-
-/*
- * Clean up preparse data.
- */
-static void rxrpc_free_preparse_s(struct key_preparsed_payload *prep)
-{
- if (prep->payload.data[0])
- crypto_free_skcipher(prep->payload.data[0]);
-}
-
/*
* dispose of the data dangling from the corpse of a rxrpc key
*/
@@ -480,17 +404,6 @@ static void rxrpc_destroy(struct key *key)
rxrpc_free_token_list(key->payload.data[0]);
}
-/*
- * dispose of the data dangling from the corpse of a rxrpc key
- */
-static void rxrpc_destroy_s(struct key *key)
-{
- if (key->payload.data[0]) {
- crypto_free_skcipher(key->payload.data[0]);
- key->payload.data[0] = NULL;
- }
-}
-
/*
* describe the rxrpc key
*/
@@ -517,14 +430,6 @@ static void rxrpc_describe(const struct key *key, struct seq_file *m)
}
}
-/*
- * describe the rxrpc server key
- */
-static void rxrpc_describe_s(const struct key *key, struct seq_file *m)
-{
- seq_puts(m, key->description);
-}
-
/*
* grab the security key for a socket
*/
@@ -555,36 +460,6 @@ int rxrpc_request_key(struct rxrpc_sock *rx, sockptr_t optval, int optlen)
return 0;
}
-/*
- * grab the security keyring for a server socket
- */
-int rxrpc_server_keyring(struct rxrpc_sock *rx, sockptr_t optval, int optlen)
-{
- struct key *key;
- char *description;
-
- _enter("");
-
- if (optlen <= 0 || optlen > PAGE_SIZE - 1)
- return -EINVAL;
-
- description = memdup_sockptr_nul(optval, optlen);
- if (IS_ERR(description))
- return PTR_ERR(description);
-
- key = request_key(&key_type_keyring, description, NULL);
- if (IS_ERR(key)) {
- kfree(description);
- _leave(" = %ld", PTR_ERR(key));
- return PTR_ERR(key);
- }
-
- rx->securities = key;
- kfree(description);
- _leave(" = 0 [key %x]", key->serial);
- return 0;
-}
-
/*
* generate a server data key
*/
diff --git a/net/rxrpc/server_key.c b/net/rxrpc/server_key.c
new file mode 100644
index 000000000000..b75bda05120d
--- /dev/null
+++ b/net/rxrpc/server_key.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* RxRPC key management
+ *
+ * Copyright (C) 2007 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * RxRPC keys should have a description of describing their purpose:
+ * "afs@CAMBRIDGE.REDHAT.COM>
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <crypto/skcipher.h>
+#include <linux/module.h>
+#include <linux/net.h>
+#include <linux/skbuff.h>
+#include <linux/key-type.h>
+#include <linux/ctype.h>
+#include <linux/slab.h>
+#include <net/sock.h>
+#include <net/af_rxrpc.h>
+#include <keys/rxrpc-type.h>
+#include <keys/user-type.h>
+#include "ar-internal.h"
+
+static int rxrpc_vet_description_s(const char *);
+static int rxrpc_preparse_s(struct key_preparsed_payload *);
+static void rxrpc_free_preparse_s(struct key_preparsed_payload *);
+static void rxrpc_destroy_s(struct key *);
+static void rxrpc_describe_s(const struct key *, struct seq_file *);
+
+/*
+ * rxrpc server defined keys take "<serviceId>:<securityIndex>" as the
+ * description and an 8-byte decryption key as the payload
+ */
+struct key_type key_type_rxrpc_s = {
+ .name = "rxrpc_s",
+ .flags = KEY_TYPE_NET_DOMAIN,
+ .vet_description = rxrpc_vet_description_s,
+ .preparse = rxrpc_preparse_s,
+ .free_preparse = rxrpc_free_preparse_s,
+ .instantiate = generic_key_instantiate,
+ .destroy = rxrpc_destroy_s,
+ .describe = rxrpc_describe_s,
+};
+
+/*
+ * Vet the description for an RxRPC server key
+ */
+static int rxrpc_vet_description_s(const char *desc)
+{
+ unsigned long num;
+ char *p;
+
+ num = simple_strtoul(desc, &p, 10);
+ if (*p != ':' || num > 65535)
+ return -EINVAL;
+ num = simple_strtoul(p + 1, &p, 10);
+ if (*p || num < 1 || num > 255)
+ return -EINVAL;
+ return 0;
+}
+
+/*
+ * Preparse a server secret key.
+ *
+ * The data should be the 8-byte secret key.
+ */
+static int rxrpc_preparse_s(struct key_preparsed_payload *prep)
+{
+ struct crypto_skcipher *ci;
+
+ _enter("%zu", prep->datalen);
+
+ if (prep->datalen != 8)
+ return -EINVAL;
+
+ memcpy(&prep->payload.data[2], prep->data, 8);
+
+ ci = crypto_alloc_skcipher("pcbc(des)", 0, CRYPTO_ALG_ASYNC);
+ if (IS_ERR(ci)) {
+ _leave(" = %ld", PTR_ERR(ci));
+ return PTR_ERR(ci);
+ }
+
+ if (crypto_skcipher_setkey(ci, prep->data, 8) < 0)
+ BUG();
+
+ prep->payload.data[0] = ci;
+ _leave(" = 0");
+ return 0;
+}
+
+static void rxrpc_free_preparse_s(struct key_preparsed_payload *prep)
+{
+ if (prep->payload.data[0])
+ crypto_free_skcipher(prep->payload.data[0]);
+}
+
+static void rxrpc_destroy_s(struct key *key)
+{
+ if (key->payload.data[0]) {
+ crypto_free_skcipher(key->payload.data[0]);
+ key->payload.data[0] = NULL;
+ }
+}
+
+static void rxrpc_describe_s(const struct key *key, struct seq_file *m)
+{
+ seq_puts(m, key->description);
+}
+
+/*
+ * grab the security keyring for a server socket
+ */
+int rxrpc_server_keyring(struct rxrpc_sock *rx, sockptr_t optval, int optlen)
+{
+ struct key *key;
+ char *description;
+
+ _enter("");
+
+ if (optlen <= 0 || optlen > PAGE_SIZE - 1)
+ return -EINVAL;
+
+ description = memdup_sockptr_nul(optval, optlen);
+ if (IS_ERR(description))
+ return PTR_ERR(description);
+
+ key = request_key(&key_type_keyring, description, NULL);
+ if (IS_ERR(key)) {
+ kfree(description);
+ _leave(" = %ld", PTR_ERR(key));
+ return PTR_ERR(key);
+ }
+
+ rx->securities = key;
+ kfree(description);
+ _leave(" = 0 [key %x]", key->serial);
+ return 0;
+}
^ permalink raw reply related
* [PATCH net-next 07/17] rxrpc: Hand server key parsing off to the security class
From: David Howells @ 2020-11-23 20:10 UTC (permalink / raw)
To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <160616220405.830164.2239716599743995145.stgit@warthog.procyon.org.uk>
Hand responsibility for parsing a server key off to the security class. We
can determine which class from the description. This is necessary as rxgk
server keys have different lookup requirements and different content
requirements (dependent on crypto type) to those of rxkad server keys.
Signed-off-by: David Howells <dhowells@redhat.com>
---
net/rxrpc/ar-internal.h | 11 +++++++++
net/rxrpc/rxkad.c | 47 +++++++++++++++++++++++++++++++++++++++
net/rxrpc/security.c | 2 +-
net/rxrpc/server_key.c | 56 +++++++++++++++++++++++------------------------
4 files changed, 86 insertions(+), 30 deletions(-)
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 6427bcfb4df5..6682c797b878 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -35,6 +35,7 @@ struct rxrpc_crypt {
#define rxrpc_queue_delayed_work(WS,D) \
queue_delayed_work(rxrpc_workqueue, (WS), (D))
+struct key_preparsed_payload;
struct rxrpc_connection;
/*
@@ -217,6 +218,15 @@ struct rxrpc_security {
/* Clean up a security service */
void (*exit)(void);
+ /* Parse the information from a server key */
+ int (*preparse_server_key)(struct key_preparsed_payload *);
+
+ /* Clean up the preparse buffer after parsing a server key */
+ void (*free_preparse_server_key)(struct key_preparsed_payload *);
+
+ /* Destroy the payload of a server key */
+ void (*destroy_server_key)(struct key *);
+
/* initialise a connection's security */
int (*init_connection_security)(struct rxrpc_connection *,
struct rxrpc_key_token *);
@@ -1050,6 +1060,7 @@ extern const struct rxrpc_security rxkad;
* security.c
*/
int __init rxrpc_init_security(void);
+const struct rxrpc_security *rxrpc_security_lookup(u8);
void rxrpc_exit_security(void);
int rxrpc_init_client_conn_security(struct rxrpc_connection *);
const struct rxrpc_security *rxrpc_get_incoming_security(struct rxrpc_sock *,
diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c
index 0d21935dac27..3057f00a6978 100644
--- a/net/rxrpc/rxkad.c
+++ b/net/rxrpc/rxkad.c
@@ -15,6 +15,7 @@
#include <linux/scatterlist.h>
#include <linux/ctype.h>
#include <linux/slab.h>
+#include <linux/key-type.h>
#include <net/sock.h>
#include <net/af_rxrpc.h>
#include <keys/rxrpc-type.h>
@@ -46,6 +47,49 @@ static struct crypto_sync_skcipher *rxkad_ci;
static struct skcipher_request *rxkad_ci_req;
static DEFINE_MUTEX(rxkad_ci_mutex);
+/*
+ * Parse the information from a server key
+ *
+ * The data should be the 8-byte secret key.
+ */
+static int rxkad_preparse_server_key(struct key_preparsed_payload *prep)
+{
+ struct crypto_skcipher *ci;
+
+ if (prep->datalen != 8)
+ return -EINVAL;
+
+ memcpy(&prep->payload.data[2], prep->data, 8);
+
+ ci = crypto_alloc_skcipher("pcbc(des)", 0, CRYPTO_ALG_ASYNC);
+ if (IS_ERR(ci)) {
+ _leave(" = %ld", PTR_ERR(ci));
+ return PTR_ERR(ci);
+ }
+
+ if (crypto_skcipher_setkey(ci, prep->data, 8) < 0)
+ BUG();
+
+ prep->payload.data[0] = ci;
+ _leave(" = 0");
+ return 0;
+}
+
+static void rxkad_free_preparse_server_key(struct key_preparsed_payload *prep)
+{
+
+ if (prep->payload.data[0])
+ crypto_free_skcipher(prep->payload.data[0]);
+}
+
+static void rxkad_destroy_server_key(struct key *key)
+{
+ if (key->payload.data[0]) {
+ crypto_free_skcipher(key->payload.data[0]);
+ key->payload.data[0] = NULL;
+ }
+}
+
/*
* initialise connection security
*/
@@ -1302,6 +1346,9 @@ const struct rxrpc_security rxkad = {
.no_key_abort = RXKADUNKNOWNKEY,
.init = rxkad_init,
.exit = rxkad_exit,
+ .preparse_server_key = rxkad_preparse_server_key,
+ .free_preparse_server_key = rxkad_free_preparse_server_key,
+ .destroy_server_key = rxkad_destroy_server_key,
.init_connection_security = rxkad_init_connection_security,
.prime_packet_security = rxkad_prime_packet_security,
.secure_packet = rxkad_secure_packet,
diff --git a/net/rxrpc/security.c b/net/rxrpc/security.c
index bef9971e15cd..50cb5f1ee0c0 100644
--- a/net/rxrpc/security.c
+++ b/net/rxrpc/security.c
@@ -55,7 +55,7 @@ void rxrpc_exit_security(void)
/*
* look up an rxrpc security module
*/
-static const struct rxrpc_security *rxrpc_security_lookup(u8 security_index)
+const struct rxrpc_security *rxrpc_security_lookup(u8 security_index)
{
if (security_index >= ARRAY_SIZE(rxrpc_security_types))
return NULL;
diff --git a/net/rxrpc/server_key.c b/net/rxrpc/server_key.c
index b75bda05120d..1a2f0b63ee1d 100644
--- a/net/rxrpc/server_key.c
+++ b/net/rxrpc/server_key.c
@@ -30,8 +30,8 @@ static void rxrpc_destroy_s(struct key *);
static void rxrpc_describe_s(const struct key *, struct seq_file *);
/*
- * rxrpc server defined keys take "<serviceId>:<securityIndex>" as the
- * description and an 8-byte decryption key as the payload
+ * rxrpc server keys take "<serviceId>:<securityIndex>[:<sec-specific>]" as the
+ * description and the key material as the payload.
*/
struct key_type key_type_rxrpc_s = {
.name = "rxrpc_s",
@@ -45,64 +45,62 @@ struct key_type key_type_rxrpc_s = {
};
/*
- * Vet the description for an RxRPC server key
+ * Vet the description for an RxRPC server key.
*/
static int rxrpc_vet_description_s(const char *desc)
{
- unsigned long num;
+ unsigned long service, sec_class;
char *p;
- num = simple_strtoul(desc, &p, 10);
- if (*p != ':' || num > 65535)
+ service = simple_strtoul(desc, &p, 10);
+ if (*p != ':' || service > 65535)
return -EINVAL;
- num = simple_strtoul(p + 1, &p, 10);
- if (*p || num < 1 || num > 255)
+ sec_class = simple_strtoul(p + 1, &p, 10);
+ if ((*p && *p != ':') || sec_class < 1 || sec_class > 255)
return -EINVAL;
return 0;
}
/*
* Preparse a server secret key.
- *
- * The data should be the 8-byte secret key.
*/
static int rxrpc_preparse_s(struct key_preparsed_payload *prep)
{
- struct crypto_skcipher *ci;
+ const struct rxrpc_security *sec;
+ unsigned int service, sec_class;
+ int n;
_enter("%zu", prep->datalen);
- if (prep->datalen != 8)
+ if (!prep->orig_description)
return -EINVAL;
- memcpy(&prep->payload.data[2], prep->data, 8);
+ if (sscanf(prep->orig_description, "%u:%u%n", &service, &sec_class, &n) != 2)
+ return -EINVAL;
- ci = crypto_alloc_skcipher("pcbc(des)", 0, CRYPTO_ALG_ASYNC);
- if (IS_ERR(ci)) {
- _leave(" = %ld", PTR_ERR(ci));
- return PTR_ERR(ci);
- }
+ sec = rxrpc_security_lookup(sec_class);
+ if (!sec)
+ return -ENOPKG;
- if (crypto_skcipher_setkey(ci, prep->data, 8) < 0)
- BUG();
+ prep->payload.data[1] = (struct rxrpc_security *)sec;
- prep->payload.data[0] = ci;
- _leave(" = 0");
- return 0;
+ return sec->preparse_server_key(prep);
}
static void rxrpc_free_preparse_s(struct key_preparsed_payload *prep)
{
- if (prep->payload.data[0])
- crypto_free_skcipher(prep->payload.data[0]);
+ const struct rxrpc_security *sec = prep->payload.data[1];
+
+ if (sec)
+ sec->free_preparse_server_key(prep);
}
static void rxrpc_destroy_s(struct key *key)
{
- if (key->payload.data[0]) {
- crypto_free_skcipher(key->payload.data[0]);
- key->payload.data[0] = NULL;
- }
+ const struct rxrpc_security *sec = key->payload.data[1];
+
+ if (sec)
+ sec->destroy_server_key(key);
}
static void rxrpc_describe_s(const struct key *key, struct seq_file *m)
^ permalink raw reply related
* [PATCH net-next 08/17] rxrpc: Don't leak the service-side session key to userspace
From: David Howells @ 2020-11-23 20:11 UTC (permalink / raw)
To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <160616220405.830164.2239716599743995145.stgit@warthog.procyon.org.uk>
Don't let someone reading a service-side rxrpc-type key get access to the
session key that was exchanged with the client. The server application
will, at some point, need to be able to read the information in the ticket,
but this probably shouldn't include the key material.
Signed-off-by: David Howells <dhowells@redhat.com>
---
include/keys/rxrpc-type.h | 1 +
net/rxrpc/key.c | 8 ++++++--
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/include/keys/rxrpc-type.h b/include/keys/rxrpc-type.h
index 8e4ced9b4ecf..333c0f49a9cd 100644
--- a/include/keys/rxrpc-type.h
+++ b/include/keys/rxrpc-type.h
@@ -36,6 +36,7 @@ struct rxkad_key {
*/
struct rxrpc_key_token {
u16 security_index; /* RxRPC header security index */
+ bool no_leak_key; /* Don't copy the key to userspace */
struct rxrpc_key_token *next; /* the next token in the list */
union {
struct rxkad_key *kad;
diff --git a/net/rxrpc/key.c b/net/rxrpc/key.c
index 3bd7b9d48d27..ed29ec01237b 100644
--- a/net/rxrpc/key.c
+++ b/net/rxrpc/key.c
@@ -579,7 +579,8 @@ static long rxrpc_read(const struct key *key,
case RXRPC_SECURITY_RXKAD:
toksize += 8 * 4; /* viceid, kvno, key*2, begin,
* end, primary, tktlen */
- toksize += RND(token->kad->ticket_len);
+ if (!token->no_leak_key)
+ toksize += RND(token->kad->ticket_len);
break;
default: /* we have a ticket we can't encode */
@@ -654,7 +655,10 @@ static long rxrpc_read(const struct key *key,
ENCODE(token->kad->start);
ENCODE(token->kad->expiry);
ENCODE(token->kad->primary_flag);
- ENCODE_DATA(token->kad->ticket_len, token->kad->ticket);
+ if (token->no_leak_key)
+ ENCODE(0);
+ else
+ ENCODE_DATA(token->kad->ticket_len, token->kad->ticket);
break;
default:
^ permalink raw reply related
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