| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 12 Oct 2021 02:36:18 +0000
From: "liujian (CE)" <liujian56@...wei.com>
To: John Fastabend <john.fastabend@...il.com>,
"daniel@...earbox.net" <daniel@...earbox.net>,
"jakub@...udflare.com" <jakub@...udflare.com>,
"lmb@...udflare.com" <lmb@...udflare.com>,
"davem@...emloft.net" <davem@...emloft.net>,
"kuba@...nel.org" <kuba@...nel.org>,
"ast@...nel.org" <ast@...nel.org>,
"andrii@...nel.org" <andrii@...nel.org>,
"kafai@...com" <kafai@...com>,
"songliubraving@...com" <songliubraving@...com>,
"yhs@...com" <yhs@...com>,
"kpsingh@...nel.org" <kpsingh@...nel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"bpf@...r.kernel.org" <bpf@...r.kernel.org>,
"xiyou.wangcong@...il.com" <xiyou.wangcong@...il.com>
Subject: RE: [PATCH v4] skmsg: lose offset info in sk_psock_skb_ingress
> -----Original Message-----
> From: liujian (CE)
> Sent: Monday, October 4, 2021 12:28 PM
> To: 'John Fastabend' <john.fastabend@...il.com>; daniel@...earbox.net;
> jakub@...udflare.com; lmb@...udflare.com; davem@...emloft.net;
> kuba@...nel.org; ast@...nel.org; andrii@...nel.org; kafai@...com;
> songliubraving@...com; yhs@...com; kpsingh@...nel.org;
> netdev@...r.kernel.org; bpf@...r.kernel.org; xiyou.wangcong@...il.com
> Subject: RE: [PATCH v4] skmsg: lose offset info in sk_psock_skb_ingress
>
>
>
> > -----Original Message-----
> > From: John Fastabend [mailto:john.fastabend@...il.com]
> > Sent: Friday, October 1, 2021 6:48 AM
> > To: liujian (CE) <liujian56@...wei.com>; john.fastabend@...il.com;
> > daniel@...earbox.net; jakub@...udflare.com; lmb@...udflare.com;
> > davem@...emloft.net; kuba@...nel.org; ast@...nel.org;
> > andrii@...nel.org; kafai@...com; songliubraving@...com; yhs@...com;
> > kpsingh@...nel.org; netdev@...r.kernel.org; bpf@...r.kernel.org;
> > xiyou.wangcong@...il.com
> > Cc: liujian (CE) <liujian56@...wei.com>
> > Subject: RE: [PATCH v4] skmsg: lose offset info in
> > sk_psock_skb_ingress
> >
> > Liu Jian wrote:
> > > If sockmap enable strparser, there are lose offset info in
> > > sk_psock_skb_ingress. If the length determined by parse_msg function
> > > is not skb->len, the skb will be converted to sk_msg multiple times,
> > > and userspace app will get the data multiple times.
> > >
> > > Fix this by get the offset and length from strp_msg.
> > > And as Cong suggestion, add one bit in skb->_sk_redir to distinguish
> > > enable or disable strparser.
> > >
> > > Signed-off-by: Liu Jian <liujian56@...wei.com>
> > > ---
> >
> > Thanks. Please add Fixes tags so we can track these I've added it here.
> >
> > This has been broken from the initial patches and after a quick glance
> > I suspect this will need manual backports if we need it. Also all the
> > I use and all the selftests set parser to a nop by returning skb->len.
> >
> > Can you also create a test so we can ensure we don't break this again?
> Okay, I will do this after the holiday.
Hi John,
I checked selftests, there are have one test case named " test_txmsg_ingress_parser".
But with this patch and ktls, the test failed, this because ktls parser(tls_read_size) return value is 285 not 256.
the case like this:
tls_sk1 --> redir_sk --> tls_sk2
tls_sk1 sent out 512 bytes data, after tls related processing redir_sk recved 570 btyes data,
and redirect 512 (skb_use_parser) bytes data to tls_sk2; but tls_sk2 needs 285 * 2 bytes data, receive timeout occurred.
I fix this as below:
--- a/tools/testing/selftests/bpf/test_sockmap.c
+++ b/tools/testing/selftests/bpf/test_sockmap.c
@@ -1680,6 +1680,8 @@ static void test_txmsg_ingress_parser(int cgrp, struct sockmap_options *opt)
{
txmsg_pass = 1;
skb_use_parser = 512;
+ if (ktls == 1)
+ skb_use_parser = 570;
opt->iov_length = 256;
opt->iov_count = 1;
opt->rate = 2;
And i add one new test as below, is it ok?
--- a/tools/testing/selftests/bpf/test_sockmap.c
+++ b/tools/testing/selftests/bpf/test_sockmap.c
@@ -139,6 +139,7 @@ struct sockmap_options {
bool sendpage;
bool data_test;
bool drop_expected;
+ bool check_recved_len;
int iov_count;
int iov_length;
int rate;
@@ -556,8 +557,12 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
int err, i, flags = MSG_NOSIGNAL;
bool drop = opt->drop_expected;
bool data = opt->data_test;
+ int iov_alloc_length = iov_length;
- err = msg_alloc_iov(&msg, iov_count, iov_length, data, tx);
+ if (!tx && opt->check_recved_len)
+ iov_alloc_length *= 2;
+
+ err = msg_alloc_iov(&msg, iov_count, iov_alloc_length, data, tx);
if (err)
goto out_errno;
if (peek_flag) {
@@ -665,6 +670,13 @@ static int msg_loop(int fd, int iov_count, int iov_length, int cnt,
s->bytes_recvd += recv;
+ if (opt->check_recved_len && s->bytes_recvd > total_bytes) {
+ errno = EMSGSIZE;
+ fprintf(stderr, "recv failed(), bytes_recvd:%zd, total_bytes:%f\n",
+ s->bytes_recvd, total_bytes);
+ goto out_errno;
+ }
+
if (data) {
int chunk_sz = opt->sendpage ?
iov_length * cnt :
@@ -744,7 +756,8 @@ static int sendmsg_test(struct sockmap_options *opt)
rxpid = fork();
if (rxpid == 0) {
- iov_buf -= (txmsg_pop - txmsg_start_pop + 1);
+ if (txmsg_pop || txmsg_start_pop)
+ iov_buf -= (txmsg_pop - txmsg_start_pop + 1);
if (opt->drop_expected || txmsg_ktls_skb_drop)
_exit(0);
@@ -1688,6 +1701,19 @@ static void test_txmsg_ingress_parser(int cgrp, struct sockmap_options *opt)
test_exec(cgrp, opt);
}
+static void test_txmsg_ingress_parser2(int cgrp, struct sockmap_options *opt)
+{
+ if (ktls == 1)
+ return;
+ skb_use_parser = 10;
+ opt->iov_length = 20;
+ opt->iov_count = 1;
+ opt->rate = 1;
+ opt->check_recved_len = true;
+ test_exec(cgrp, opt);
+ opt->check_recved_len = false;
+}
+
char *map_names[] = {
"sock_map",
"sock_map_txmsg",
@@ -1786,7 +1812,8 @@ struct _test test[] = {
{"txmsg test pull-data", test_txmsg_pull},
{"txmsg test pop-data", test_txmsg_pop},
{"txmsg test push/pop data", test_txmsg_push_pop},
- {"txmsg text ingress parser", test_txmsg_ingress_parser},
+ {"txmsg test ingress parser", test_txmsg_ingress_parser},
+ {"txmsg test ingress parser2", test_txmsg_ingress_parser2},
};
> >
> > Fixes: 604326b41a6fb ("bpf, sockmap: convert to generic sk_msg
> > interface")
> > Acked-by: John Fastabend <john.fastabend@...il.com>
> Thank you for reviewing this patch again.
Powered by blists - more mailing lists