On Sat, 23 Dec 2023 at 09:29, Simon Horman <horms@xxxxxxxxxx> wrote:
>
>
> if (data[0] == 0) {
> /* It may be a server list. */
> - if (datalen <= sizeof(*bin))
> + if (datalen <= sizeof(*v1))
> return -EINVAL;
>
> bin = (const struct dns_payload_header *)data;
Ugh, I hate how it checks the size of a *different* structure than the
one it then assigns the pointer to.
So I get the feeling that we should get rid of 'bin' entirely, and
just use the 'v1' pointer, since it literally checks that that is what
it is, and then the size check matches the thing we're casting things
to.
So then "bin->xyz" becomes "v1->hdr.xyz".
Yes, the patch becomes a bit bigger, but I think the end result is a
whole lot more obvious and maintainable.
I'd also move the remaining 'v1' variable declaration to the inner
context where it's actually used.
IOW, I personally would be much happier with a patch like the attached, but I
(a) don't want to take credit for this, since my change is purely syntactic
(b) have not tested this patch apart from checking that it compiles
in at least one config
so honestly, I'd love to see this patch come back to me with sign-offs
and tested-bys by the actual people who noticed this.
Hmm?
Linus
net/dns_resolver/dns_key.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/net/dns_resolver/dns_key.c b/net/dns_resolver/dns_key.c
index 2a6d363763a2..f18ca02aa95a 100644
--- a/net/dns_resolver/dns_key.c
+++ b/net/dns_resolver/dns_key.c
@@ -91,8 +91,6 @@ const struct cred *dns_resolver_cache;
static int
dns_resolver_preparse(struct key_preparsed_payload *prep)
{
- const struct dns_server_list_v1_header *v1;
- const struct dns_payload_header *bin;
struct user_key_payload *upayload;
unsigned long derrno;
int ret;
@@ -103,27 +101,28 @@ dns_resolver_preparse(struct key_preparsed_payload *prep)
return -EINVAL;
if (data[0] == 0) {
+ const struct dns_server_list_v1_header *v1;
+
/* It may be a server list. */
- if (datalen <= sizeof(*bin))
+ if (datalen <= sizeof(*v1))
return -EINVAL;
- bin = (const struct dns_payload_header *)data;
- kenter("[%u,%u],%u", bin->content, bin->version, datalen);
- if (bin->content != DNS_PAYLOAD_IS_SERVER_LIST) {
+ v1 = (const struct dns_server_list_v1_header *)data;
+ kenter("[%u,%u],%u", v1->hdr.content, v1->hdr.version, datalen);
+ if (v1->hdr.content != DNS_PAYLOAD_IS_SERVER_LIST) {
pr_warn_ratelimited(
"dns_resolver: Unsupported content type (%u)\n",
- bin->content);
+ v1->hdr.content);
return -EINVAL;
}
- if (bin->version != 1) {
+ if (v1->hdr.version != 1) {
pr_warn_ratelimited(
"dns_resolver: Unsupported server list version (%u)\n",
- bin->version);
+ v1->hdr.version);
return -EINVAL;
}
- v1 = (const struct dns_server_list_v1_header *)bin;
if ((v1->status != DNS_LOOKUP_GOOD &&
v1->status != DNS_LOOKUP_GOOD_WITH_BAD)) {
if (prep->expiry == TIME64_MAX)
