* static inline int xfrm_mark_get() broken
@ 2010-06-28 18:46 Andreas Steffen
2010-06-30 4:46 ` Simon Horman
0 siblings, 1 reply; 4+ messages in thread
From: Andreas Steffen @ 2010-06-28 18:46 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 761 bytes --]
Hi,
experimenting with the new XFRM_MARK feature of the 2.6.34 kernel
I found out that the extraction of the mark mask might accidentally
work on 64 bit platforms but on 32 bit platforms the function is
awfully broken. The rather trivial patch attached to this mail fixes
the problem. Otherwise the XFRM_MARK feature seems quite promising!
Best regards
Andreas
======================================================================
Andreas Steffen e-mail: andreas.steffen@hsr.ch
Institute for Internet Technologies and Applications
Hochschule fuer Technik Rapperswil phone: +41 55 222 42 68
CH-8640 Rapperswil (Switzerland) mobile: +41 76 340 25 56
===========================================================[ITA-HSR]==
[-- Attachment #2: xfrm.h.diff --]
[-- Type: text/x-patch, Size: 413 bytes --]
--- linux/include/net/xfrm.h.ori 2010-06-28 18:53:28.229489876 +0200
+++ linux/include/net/xfrm.h 2010-06-28 18:53:50.745487383 +0200
@@ -1587,7 +1587,7 @@
static inline int xfrm_mark_get(struct nlattr **attrs, struct xfrm_mark *m)
{
if (attrs[XFRMA_MARK])
- memcpy(m, nla_data(attrs[XFRMA_MARK]), sizeof(m));
+ memcpy(m, nla_data(attrs[XFRMA_MARK]), sizeof(struct xfrm_mark));
else
m->v = m->m = 0;
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: static inline int xfrm_mark_get() broken
2010-06-28 18:46 static inline int xfrm_mark_get() broken Andreas Steffen
@ 2010-06-30 4:46 ` Simon Horman
2010-06-30 5:03 ` Andreas Steffen
0 siblings, 1 reply; 4+ messages in thread
From: Simon Horman @ 2010-06-30 4:46 UTC (permalink / raw)
To: Andreas Steffen; +Cc: netdev
On Mon, Jun 28, 2010 at 08:46:49PM +0200, Andreas Steffen wrote:
> Hi,
>
> experimenting with the new XFRM_MARK feature of the 2.6.34 kernel
> I found out that the extraction of the mark mask might accidentally
> work on 64 bit platforms but on 32 bit platforms the function is
> awfully broken. The rather trivial patch attached to this mail fixes
> the problem. Otherwise the XFRM_MARK feature seems quite promising!
>
> Best regards
>
> Andreas
>
> ======================================================================
> Andreas Steffen e-mail: andreas.steffen@hsr.ch
> Institute for Internet Technologies and Applications
> Hochschule fuer Technik Rapperswil phone: +41 55 222 42 68
> CH-8640 Rapperswil (Switzerland) mobile: +41 76 340 25 56
> ===========================================================[ITA-HSR]==
> --- linux/include/net/xfrm.h.ori 2010-06-28 18:53:28.229489876 +0200
> +++ linux/include/net/xfrm.h 2010-06-28 18:53:50.745487383 +0200
> @@ -1587,7 +1587,7 @@
> static inline int xfrm_mark_get(struct nlattr **attrs, struct xfrm_mark *m)
> {
> if (attrs[XFRMA_MARK])
> - memcpy(m, nla_data(attrs[XFRMA_MARK]), sizeof(m));
> + memcpy(m, nla_data(attrs[XFRMA_MARK]), sizeof(struct xfrm_mark));
This fix looks correct to me, but
I believe that sizeof(*m) is the preferred style.
> else
> m->v = m->m = 0;
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: static inline int xfrm_mark_get() broken
2010-06-30 4:46 ` Simon Horman
@ 2010-06-30 5:03 ` Andreas Steffen
2010-06-30 7:01 ` Simon Horman
0 siblings, 1 reply; 4+ messages in thread
From: Andreas Steffen @ 2010-06-30 5:03 UTC (permalink / raw)
To: Simon Horman
Cc: Steffen Andreas (asteffen@hsr.ch), netdev@vger.kernel.org, jamal
Hello Simon,
actually I don't care how this bug is going to be fixed, but with
sizeof(struct xfrm_mark) I'm dead certain that both the mark
value and mask are being copied. Actually in the next inline
function right below sizeof(struct xfrm_mark) is used, too:
static inline int xfrm_mark_put(struct sk_buff *skb, struct xfrm_mark *m)
{
if (m->m | m->v)
NLA_PUT(skb, XFRMA_MARK, sizeof(struct xfrm_mark), m);
return 0;
Regards
Andreas
On 06/30/2010 06:46 AM, Simon Horman wrote:
> On Mon, Jun 28, 2010 at 08:46:49PM +0200, Andreas Steffen wrote:
>> Hi,
>>
>> experimenting with the new XFRM_MARK feature of the 2.6.34 kernel
>> I found out that the extraction of the mark mask might accidentally
>> work on 64 bit platforms but on 32 bit platforms the function is
>> awfully broken. The rather trivial patch attached to this mail fixes
>> the problem. Otherwise the XFRM_MARK feature seems quite promising!
>>
>> Best regards
>>
>> Andreas
>>
>> ======================================================================
>> Andreas Steffen e-mail: andreas.steffen@hsr.ch
>> Institute for Internet Technologies and Applications
>> Hochschule fuer Technik Rapperswil phone: +41 55 222 42 68
>> CH-8640 Rapperswil (Switzerland) mobile: +41 76 340 25 56
>> ===========================================================[ITA-HSR]==
>
>> --- linux/include/net/xfrm.h.ori 2010-06-28 18:53:28.229489876 +0200
>> +++ linux/include/net/xfrm.h 2010-06-28 18:53:50.745487383 +0200
>> @@ -1587,7 +1587,7 @@
>> static inline int xfrm_mark_get(struct nlattr **attrs, struct xfrm_mark *m)
>> {
>> if (attrs[XFRMA_MARK])
>> - memcpy(m, nla_data(attrs[XFRMA_MARK]), sizeof(m));
>> + memcpy(m, nla_data(attrs[XFRMA_MARK]), sizeof(struct xfrm_mark));
>
> This fix looks correct to me, but
> I believe that sizeof(*m) is the preferred style.
>
>> else
>> m->v = m->m = 0;
======================================================================
Andreas Steffen e-mail: andreas.steffen@hsr.ch
Institute for Internet Technologies and Applications
Hochschule fuer Technik Rapperswil phone: +41 55 222 42 68
CH-8640 Rapperswil (Switzerland) mobile: +41 76 340 25 56
===========================================================[ITA-HSR]==
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: static inline int xfrm_mark_get() broken
2010-06-30 5:03 ` Andreas Steffen
@ 2010-06-30 7:01 ` Simon Horman
0 siblings, 0 replies; 4+ messages in thread
From: Simon Horman @ 2010-06-30 7:01 UTC (permalink / raw)
To: Andreas Steffen
Cc: Steffen Andreas (asteffen@hsr.ch), netdev@vger.kernel.org, jamal
On Wed, Jun 30, 2010 at 07:03:05AM +0200, Andreas Steffen wrote:
> Hello Simon,
>
> actually I don't care how this bug is going to be fixed, but with
> sizeof(struct xfrm_mark) I'm dead certain that both the mark
> value and mask are being copied. Actually in the next inline
> function right below sizeof(struct xfrm_mark) is used, too:
>
> static inline int xfrm_mark_put(struct sk_buff *skb, struct xfrm_mark *m)
> {
> if (m->m | m->v)
> NLA_PUT(skb, XFRMA_MARK, sizeof(struct xfrm_mark), m);
> return 0;
In that case I withdraw my suggestion.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-06-30 7:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-28 18:46 static inline int xfrm_mark_get() broken Andreas Steffen
2010-06-30 4:46 ` Simon Horman
2010-06-30 5:03 ` Andreas Steffen
2010-06-30 7:01 ` Simon Horman
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 NNTP newsgroup(s).