From: Samir Bellabes <sam@synack•fr>
To: Patrick McHardy <kaber@trash•net>
Cc: linux-security-module@vger•kernel.org, jamal <hadi@cyberus•ca>,
Evgeniy Polyakov <zbr@ioremap•net>,
Neil Horman <nhorman@tuxdriver•com>,
netdev@vger•kernel.org, netfilter-devel@vger•kernel.org
Subject: Re: [RFC 4/9] snet: introduce snet_core.c and snet.h
Date: Wed, 06 Jan 2010 20:46:35 +0100 [thread overview]
Message-ID: <m2y6kbm2xw.fsf@ssh.synack.fr> (raw)
In-Reply-To: <4B41FE9D.2070708@trash.net> (Patrick McHardy's message of "Mon, 04 Jan 2010 15:43:41 +0100")
Patrick McHardy <kaber@trash•net> writes:
>> +struct snet_event {
>> + enum snet_syscall syscall;
>> + u8 protocol;
>> +} __attribute__ ((packed));
>
> Does this really need to be packed? You're using it in a
> struct snet_event_entry, which is padded anyways.
I think that that members needs to be aligned, because this struct is
used for the jhash() computation.
when testing on x86_64, I discovered that jhash value wasn't the same,
for a same struct snet_event. Then I thougth about misaligned data, and
possible 'hole' between syscall and protocol.
anyway, I patched the code to use jhash_2words() or jhash_1word().
here is a patch, which apply on top of initial serie
thank you Patrick,
sam
commit bacdb6b62549b58370298f89f908d66c5a3cab66
Author: Samir Bellabes <sam@synack•fr>
Date: Wed Jan 6 20:36:06 2010 +0100
snet : delete attribute packed for snet_event and use proper jash functions
the structure snet_event was using the attribute packed, which seems to be
unnecessary.
this patch also fix the good compute of hashes, by moving jash() to
jash_1word() and jash_2words()
Noticed by Patrick McHardy <kaber@trash•net>
Signed-off-by: Samir Bellabes <sam@synack•fr>
diff --git a/security/snet/include/snet.h b/security/snet/include/snet.h
index 47da614..75b32d5 100644
--- a/security/snet/include/snet.h
+++ b/security/snet/include/snet.h
@@ -9,6 +9,6 @@
struct snet_event {
enum snet_syscall syscall;
u8 protocol;
-} __attribute__ ((packed));
+};
#endif /* _SNET_H */
diff --git a/security/snet/snet_event.c b/security/snet/snet_event.c
index 9f0b0f3..cc3b6a2 100644
--- a/security/snet/snet_event.c
+++ b/security/snet/snet_event.c
@@ -24,17 +24,12 @@ static struct snet_event_entry *__snet_event_lookup(const enum snet_syscall sysc
unsigned int h = 0;
struct list_head *l;
struct snet_event_entry *s;
- struct snet_event t;
if (!snet_evh)
return NULL;
- /* building the element to look for */
- t.syscall = syscall;
- t.protocol = protocol;
-
/* computing its hash value */
- h = jhash(&t, sizeof(struct snet_event), 0) % snet_evh_size;
+ h = jhash_2words(syscall, protocol, 0) % snet_evh_size;
l = &snet_evh[h];
list_for_each_entry(s, l, list) {
@@ -127,7 +122,7 @@ int snet_event_insert(const enum snet_syscall syscall, const u8 protocol)
data->se.syscall = syscall;
data->se.protocol = protocol;
INIT_LIST_HEAD(&(data->list));
- h = jhash(&(data->se), sizeof(struct snet_event), 0) % snet_evh_size;
+ h = jhash_2words(data->se.syscall, data->se.protocol, 0) % snet_evh_size;
list_add_tail(&data->list, &snet_evh[h]);
write_unlock_bh(&snet_evh_lock);
pr_debug("[%u]=(syscall=%s, protocol=%u)\n",
diff --git a/security/snet/snet_verdict.c b/security/snet/snet_verdict.c
index 0cad06b..477af3b 100644
--- a/security/snet/snet_verdict.c
+++ b/security/snet/snet_verdict.c
@@ -29,18 +29,16 @@ static struct snet_verdict_entry *__snet_verdict_lookup(const u32 verdict_id)
unsigned int h = 0;
struct list_head *l = NULL;
struct snet_verdict_entry *s = NULL;
- u32 vid = 0;
if (!snet_vdh)
return NULL;
- vid = verdict_id;
/* computing its hash value */
- h = jhash(&vid, sizeof(u32), 0) % snet_vdh_size;
+ h = jhash_1word(verdict_id, 0) % snet_vdh_size;
l = &snet_vdh[h];
list_for_each_entry(s, l, list) {
- if (s->verdict_id == vid) {
+ if (s->verdict_id == verdict_id) {
return s;
}
}
@@ -134,7 +132,7 @@ int snet_verdict_insert(void)
data->verdict_id = verdict_id;
data->verdict = SNET_VERDICT_PENDING;
INIT_LIST_HEAD(&(data->list));
- h = jhash(&(data->verdict_id), sizeof(u32), 0) % snet_vdh_size;
+ h = jhash_1word(data->verdict_id, 0) % snet_vdh_size;
write_lock_bh(&snet_vdh_lock);
if (snet_vdh) {
next prev parent reply other threads:[~2010-01-06 19:46 UTC|newest]
Thread overview: 61+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-01-02 13:04 [RFC 0/9] snet: Security for NETwork syscalls Samir Bellabes
2010-01-02 13:04 ` [RFC 1/9] lsm: add security_socket_closed() Samir Bellabes
2010-01-04 18:33 ` Serge E. Hallyn
2010-01-02 13:04 ` [RFC 2/9] Revert "lsm: Remove the socket_post_accept() hook" Samir Bellabes
2010-01-04 18:36 ` Serge E. Hallyn
2010-01-05 0:31 ` Tetsuo Handa
2010-01-05 0:38 ` Serge E. Hallyn
2010-01-02 13:04 ` [RFC 3/9] snet: introduce security/snet, Makefile and Kconfig changes Samir Bellabes
2010-01-04 18:39 ` Serge E. Hallyn
2010-01-06 6:04 ` Samir Bellabes
2010-01-02 13:04 ` [RFC 4/9] snet: introduce snet_core.c and snet.h Samir Bellabes
2010-01-04 14:43 ` Patrick McHardy
2010-01-06 18:23 ` Samir Bellabes
2010-01-06 19:46 ` Samir Bellabes [this message]
2010-01-06 19:58 ` Evgeniy Polyakov
2010-01-23 2:07 ` Samir Bellabes
2010-01-23 2:18 ` Evgeniy Polyakov
2010-01-07 14:34 ` Samir Bellabes
2010-01-07 14:53 ` Samir Bellabes
2010-01-07 14:58 ` Samir Bellabes
2010-01-08 4:32 ` Samir Bellabes
2010-01-04 18:42 ` Serge E. Hallyn
2010-01-06 6:12 ` Samir Bellabes
2010-01-02 13:04 ` [RFC 5/9] snet: introduce snet_event.c and snet_event.h Samir Bellabes
2010-01-02 20:09 ` Evgeniy Polyakov
2010-01-02 23:38 ` Samir Bellabes
2010-01-04 19:08 ` Serge E. Hallyn
2010-01-08 7:21 ` Samir Bellabes
2010-01-08 15:34 ` Serge E. Hallyn
2010-01-08 17:44 ` Samir Bellabes
2010-01-08 17:51 ` Samir Bellabes
2010-01-08 18:10 ` Serge E. Hallyn
2010-01-02 13:04 ` [RFC 6/9] snet: introduce snet_hooks.c and snet_hook.h Samir Bellabes
2010-01-02 20:13 ` Evgeniy Polyakov
2010-01-03 11:10 ` Samir Bellabes
2010-01-03 19:16 ` Stephen Hemminger
2010-01-03 22:26 ` Samir Bellabes
2010-01-02 13:04 ` [RFC 7/9] snet: introduce snet_netlink.c and snet_netlink.h Samir Bellabes
2010-01-04 15:08 ` Patrick McHardy
2010-01-13 4:19 ` Samir Bellabes
2010-01-13 4:28 ` Samir Bellabes
2010-01-13 5:36 ` Patrick McHardy
2010-01-13 4:36 ` Samir Bellabes
2010-01-13 4:41 ` Samir Bellabes
2010-01-13 6:03 ` Samir Bellabes
2010-01-13 6:20 ` Samir Bellabes
2010-01-15 7:02 ` Samir Bellabes
2010-01-15 9:15 ` Samir Bellabes
2010-01-16 1:59 ` Samir Bellabes
2010-01-17 5:42 ` Samir Bellabes
2010-01-23 19:33 ` Samir Bellabes
2010-01-02 13:04 ` [RFC 8/9] snet: introduce snet_verdict.c and snet_verdict.h Samir Bellabes
2010-01-02 13:04 ` [RFC 9/9] snet: introduce snet_utils.c and snet_utils.h Samir Bellabes
2010-01-03 16:57 ` [RFC 0/9] snet: Security for NETwork syscalls jamal
2010-01-05 7:26 ` Samir Bellabes
2010-01-05 8:20 ` Tetsuo Handa
2010-01-05 14:09 ` Serge E. Hallyn
2010-01-06 0:23 ` [PATCH] LSM: Update comment on security_sock_rcv_skb Tetsuo Handa
2010-01-06 3:27 ` Serge E. Hallyn
2010-01-10 21:53 ` James Morris
2010-01-10 16:20 ` [RFC 0/9] snet: Security for NETwork syscalls jamal
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=m2y6kbm2xw.fsf@ssh.synack.fr \
--to=sam@synack$(echo .)fr \
--cc=hadi@cyberus$(echo .)ca \
--cc=kaber@trash$(echo .)net \
--cc=linux-security-module@vger$(echo .)kernel.org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=netfilter-devel@vger$(echo .)kernel.org \
--cc=nhorman@tuxdriver$(echo .)com \
--cc=zbr@ioremap$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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