From: Denis Kenzior <denkenz@gmail.com>
To: jeremy.whiting@collabora.com, iwd@lists.linux.dev
Cc: ed.smith@collabora.com, alvaro.soliverez@collabora.com
Subject: Re: [PATCH] Register EAPOL frame listeners earlier
Date: Wed, 27 Mar 2024 16:01:37 -0500 [thread overview]
Message-ID: <fd511498-2a6c-4646-85b3-4f2f3f6f379b@gmail.com> (raw)
In-Reply-To: <20240327184927.677804-1-jeremy.whiting@collabora.com>
Hi Jeremy/Ed,
On 3/27/24 13:49, jeremy.whiting@collabora.com wrote:
> From: Ed Smith <ed.smith@collabora.com>
>
> If we register the main EAPOL frame listener as late as the associate
> event, it may not observe ptk_1_of_4. This defeats handling for early
> messages in eapol_rx_packet, which only sees messages once it has been
> registered.
>
> If we move registration to the authenticate event, then the EAPOL
> frame listeners should observe all messages, without any possible
> races. Note that the messages are not actually processed until
> eapol_start() is called, and we haven't moved that call site. All
> that's changing here is how early EAPOL messages can be observed.
> ---
> src/netdev.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
Can we please use patch versioning? I can't keep track which version of this
patch I'm looking at now.
> diff --git a/src/netdev.c b/src/netdev.c
> index 09fac959..886a85f5 100644
> --- a/src/netdev.c
> +++ b/src/netdev.c
> @@ -2896,6 +2896,14 @@ static bool kernel_will_retry_auth(uint16_t status_code,
> return false;
> }
>
> +static void netdev_ensure_registered(struct netdev *netdev)
The naming could use some work. What is being registered?
> +{
> + if (!netdev->sm) {
I would do:
if (L_WARN_ON(netdev->sm))
return;
....
> + netdev->sm = eapol_sm_new(netdev->handshake);
> + eapol_register(netdev->sm);
> + }
> +}
> +
> static void netdev_authenticate_event(struct l_genl_msg *msg,
> struct netdev *netdev)
> {
> @@ -2982,8 +2990,10 @@ static void netdev_authenticate_event(struct l_genl_msg *msg,
> NULL, netdev->user_data);
>
> /* We have sent another CMD_AUTHENTICATE / CMD_ASSOCIATE */
> - if (ret == 0 || ret == -EAGAIN)
> + if (ret == 0 || ret == -EAGAIN) {
> + netdev_ensure_registered(netdev);
Nit: The whole point of EAGAIN is that we're not proceeding to the next stage.
See src/auth-proto.h. You should only be trying to create eapol_sm when we do.
> return;
> + }
>
> retry = kernel_will_retry_auth(status_code,
> L_CPU_TO_LE16(auth->algorithm),
Regards,
-Denis
next prev parent reply other threads:[~2024-03-27 21:01 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-27 18:49 [PATCH] Register EAPOL frame listeners earlier jeremy.whiting
2024-03-27 21:01 ` Denis Kenzior [this message]
-- strict thread matches above, loose matches on Subject: below --
2024-03-26 23:11 jeremy.whiting
2024-03-27 11:51 ` James Prestwood
2024-03-27 18:53 ` Jeremy Whiting
2024-03-27 19:10 ` James Prestwood
2024-03-25 23:41 jeremy.whiting
2024-03-26 0:00 ` Jeremy Whiting
2024-03-26 4:42 ` Denis Kenzior
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fd511498-2a6c-4646-85b3-4f2f3f6f379b@gmail.com \
--to=denkenz@gmail.com \
--cc=alvaro.soliverez@collabora.com \
--cc=ed.smith@collabora.com \
--cc=iwd@lists.linux.dev \
--cc=jeremy.whiting@collabora.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).