Skip to content

Commit 75d2d28

Browse files
committed
socket: move frame creation after memory check in quic_sendmsg
It is better to create frames only after checking memory availability to avoid freeing a frame if the memory check fails. This patch updates the code accordingly for both handshake and datagram message sending. Since handshake data may be fragmented into multiple messages, it must first use len = 1 to check the wspace and calculate msg_len with wspace considered in quic_frame_crypto_create(), then perform a second wmem check after frame creation with the real msg_len, similar as what it does for stream data. Meanwhile, duplicate code in quic_frame_crypto_create() is removed. Reported-by: Stefan Metzmacher <metze@samba.org> Signed-off-by: Xin Long <lucien.xin@gmail.com>
1 parent 3818f56 commit 75d2d28

File tree

2 files changed

+25
-42
lines changed

2 files changed

+25
-42
lines changed

modules/net/quic/frame.c

Lines changed: 4 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ static struct quic_frame *quic_frame_handshake_done_create(struct sock *sk, void
342342

343343
static struct quic_frame *quic_frame_crypto_create(struct sock *sk, void *data, u8 type)
344344
{
345-
u32 msg_len, max_frame_len, hlen = 1;
345+
u32 msg_len, max_frame_len, wspace, hlen = 1;
346346
struct quic_msginfo *info = data;
347347
struct quic_crypto *crypto;
348348
struct quic_frame *frame;
@@ -352,35 +352,13 @@ static struct quic_frame *quic_frame_crypto_create(struct sock *sk, void *data,
352352
max_frame_len = quic_packet_max_payload(quic_packet(sk));
353353
crypto = quic_crypto(sk, info->level);
354354
msg_len = iov_iter_count(info->msg);
355-
356-
if (!info->level) {
357-
offset = 0;
358-
hlen += quic_var_len(offset);
359-
hlen += quic_var_len(max_frame_len);
360-
if (msg_len > max_frame_len - hlen)
361-
return NULL;
362-
363-
frame = quic_frame_alloc(msg_len + hlen, NULL, GFP_ATOMIC);
364-
if (!frame)
365-
return NULL;
366-
p = quic_put_var(frame->data, type);
367-
p = quic_put_var(p, offset);
368-
p = quic_put_var(p, msg_len);
369-
if (!quic_frame_copy_from_iter_full(p, msg_len, info->msg)) {
370-
quic_frame_put(frame);
371-
return NULL;
372-
}
373-
p += msg_len;
374-
frame->bytes = (u16)msg_len;
375-
frame->len = (u16)(p - frame->data);
376-
frame->size = frame->len;
377-
378-
return frame;
379-
}
355+
wspace = sk_stream_wspace(sk);
380356

381357
offset = quic_crypto_send_offset(crypto);
382358
hlen += quic_var_len(offset);
383359
hlen += quic_var_len(max_frame_len);
360+
if (msg_len > wspace)
361+
msg_len = wspace;
384362
if (msg_len > max_frame_len - hlen)
385363
msg_len = max_frame_len - hlen;
386364

modules/net/quic/socket.c

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -727,31 +727,36 @@ static int quic_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len)
727727
msginfo.level = hinfo.crypto_level;
728728
msginfo.msg = &msg->msg_iter;
729729
while (iov_iter_count(&msg->msg_iter) > 0) {
730-
frame = quic_frame_create(sk, QUIC_FRAME_CRYPTO, &msginfo);
731-
if (!frame) {
732-
if (!bytes) {
733-
err = -ENOMEM;
734-
goto err;
735-
}
736-
goto out;
737-
}
738-
len = frame->bytes;
739730
if (sk_stream_wspace(sk) < len || !sk_wmem_schedule(sk, len)) {
740731
if (delay) {
741732
quic_outq_set_force_delay(outq, 0);
742733
quic_outq_transmit(sk);
743734
}
744735
err = quic_wait_for_send(sk, flags, len);
745736
if (err) {
746-
quic_frame_put(frame);
747737
if (err == -EPIPE || !bytes)
748738
goto err;
749739
goto out;
750740
}
751741
}
742+
frame = quic_frame_create(sk, QUIC_FRAME_CRYPTO, &msginfo);
743+
if (!frame) {
744+
if (!bytes) {
745+
err = -ENOMEM;
746+
goto err;
747+
}
748+
goto out;
749+
}
750+
len = frame->bytes;
751+
if (!sk_wmem_schedule(sk, len)) {
752+
iov_iter_revert(msginfo.msg, len);
753+
quic_frame_put(frame);
754+
continue;
755+
}
752756
bytes += frame->bytes;
753757
quic_outq_set_force_delay(outq, delay);
754758
quic_outq_ctrl_tail(sk, frame, delay);
759+
len = 1;
755760
}
756761
goto out;
757762
}
@@ -766,19 +771,19 @@ static int quic_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len)
766771
err = -EINVAL;
767772
goto err;
768773
}
769-
frame = quic_frame_create(sk, QUIC_FRAME_DATAGRAM_LEN, &msg->msg_iter);
770-
if (!frame) {
771-
err = -EINVAL;
772-
goto err;
773-
}
774-
len = frame->bytes;
774+
len = iov_iter_count(&msg->msg_iter);
775775
if (sk_stream_wspace(sk) < len || !sk_wmem_schedule(sk, len)) {
776776
err = quic_wait_for_send(sk, flags, len);
777777
if (err) {
778778
quic_frame_put(frame);
779779
goto err;
780780
}
781781
}
782+
frame = quic_frame_create(sk, QUIC_FRAME_DATAGRAM_LEN, &msg->msg_iter);
783+
if (!frame) {
784+
err = -EINVAL;
785+
goto err;
786+
}
782787
bytes += frame->bytes;
783788
quic_outq_set_force_delay(outq, delay);
784789
quic_outq_dgram_tail(sk, frame, delay);

0 commit comments

Comments
 (0)