(cc stable)
On Wed, 1 Apr 2009 15:50:51 -0400
Chuck Lever <chuck.lever@xxxxxxxxxx> wrote:
> >>
> >> Perhaps the solution is to malloc the nsm_private data and sm_priv
> >> is then a
> >> pointer to this data. This would guarantee the correct alignment.
> >>
> >
> > nsm_private is:
> >
> > struct nsm_private {
> > unsigned char data[SM_PRIV_SIZE];
> > };
> >
> > so the compiler is permitted to byte-align this.
> >
> > I assume that some code somewhere is accessing this with a
> > larger-than-one-byte typecast?
>
> nsm_init_private().
>
> A patch just went upstream for 2.6.30 that addresses that
commit ad5b365c1266b0c9e8e254a3c1cc4ef66bf33cba
Author: Mans Rullgard <mans@xxxxxxxxx>
AuthorDate: Sat Mar 28 19:55:20 2009 +0000
Commit: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
CommitDate: Wed Apr 1 13:24:14 2009 -0400
NSM: Fix unaligned accesses in nsm_init_private()
This fixes unaligned accesses in nsm_init_private() when
creating nlm_reboot keys.
Signed-off-by: Mans Rullgard <mans@xxxxxxxxx>
Reviewed-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
Signed-off-by: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
diff --git a/fs/lockd/mon.c b/fs/lockd/mon.c
index 5e2c4d5..6d5d4a4 100644
--- a/fs/lockd/mon.c
+++ b/fs/lockd/mon.c
@@ -16,6 +16,8 @@
#include <linux/sunrpc/svc.h>
#include <linux/lockd/lockd.h>
+#include <asm/unaligned.h>
+
#define NLMDBG_FACILITY NLMDBG_MONITOR
#define NSM_PROGRAM 100024
#define NSM_VERSION 1
@@ -274,10 +276,12 @@ static void nsm_init_private(struct nsm_handle *nsm)
{
u64 *p = (u64 *)&nsm->sm_priv.data;
struct timespec ts;
+ s64 ns;
ktime_get_ts(&ts);
- *p++ = timespec_to_ns(&ts);
- *p = (unsigned long)nsm;
+ ns = timespec_to_ns(&ts);
+ put_unaligned(ns, p);
+ put_unaligned((unsigned long)nsm, p + 1);
}
static struct nsm_handle *nsm_create_handle(const struct sockaddr *sap,
Is this the best way of fixing it? We'll crash again if some other
code starts to access .data with non-byte-sized accesses. What makes
this especially risky is that the code will pass testing on x86.
__aligned would be one way. But it would be far far cleaner to just do
--- a/include/linux/lockd/xdr.h~a
+++ a/include/linux/lockd/xdr.h
@@ -17,7 +17,7 @@
#define SM_PRIV_SIZE 16
struct nsm_private {
- unsigned char data[SM_PRIV_SIZE];
+ u64 data[SM_PRIV_SIZE/sizeof(u64)];
};
struct svc_rqst;
all the typecasting and the put_unaligned()s just go away then.
> but did not mention that it fixed an oops.
That was a fairly significant failing. Guys, please please do remember
that we're also maintaining the stable kernels. When merging a patch
please always think whether it needs to be backported.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
