Date: Wed, 17 Feb 2010 10:00:04 -0700 (MST) From: "M. Warner Losh" <imp@bsdimp.com> To: ticso@cicely.de, ticso@cicely7.cicely.de Cc: rpaulo@gmail.com, freebsd-arm@FreeBSD.org Subject: Re: kdump on ARM Message-ID: <20100217.100004.321689434032786752.imp@bsdimp.com> In-Reply-To: <20100217152900.GX43625@cicely7.cicely.de> References: <20100217151607.GU43625@cicely7.cicely.de> <20100217151941.GV43625@cicely7.cicely.de> <20100217152900.GX43625@cicely7.cicely.de>
next in thread | previous in thread | raw e-mail | index | archive | help
In message: <20100217152900.GX43625@cicely7.cicely.de>
Bernd Walter <ticso@cicely7.cicely.de> writes:
: On Wed, Feb 17, 2010 at 04:19:41PM +0100, Bernd Walter wrote:
: > On Wed, Feb 17, 2010 at 04:16:07PM +0100, Bernd Walter wrote:
: > > On Wed, Feb 17, 2010 at 02:54:05PM +0000, Rui Paulo wrote:
: > > > On 17 Feb 2010, at 14:18, Grzegorz Bernacki wrote:
: > > > I wonder if this can't be made non arm conditional?
: >
: > Ups - I'd just recovered from Mr. Sandman's work.
: > So we all agree about.
: > Nevertheless it should be verified if this is just a faulty struct
: > definition.
: > On the other hand I think it is not because someone else wrote it is
: > a brokem on mips as well.
:
: I'm really still sleeping - noone mentioned mips at all.
: > > Either this struct is properly aligned or not.
: > > So why should this be made conditional?
: > > Non strict alignment architecturs also have problems with this, but
: > > it is usualy just speed penalties.
: > > There is one ARM sepcific struct missalignment problem.
: > > In this case we usually add __packed macro to structure definition.
: > > For most structures this usually means no change on other
: > > archtitectures and we only declare the struct to forcibly be what the
: > > programmer already expected.
: > > Only a few programmers are aware that they expect something from
: > > structures, which is not garantied.
This code is clearly nutso when it comes to alignment. I've come up
with a slightly better patch. I'd though about doing the structure
assignment that I suggested in a prior note, but the compiler is free
to assume alignment when copying the structures, which may end badly.
There's no way we can add __packed or __aligned easily to this code
(although the ktrstat and ktrsockaddr routines should be able to have
that annotation, a quick test suggests that the annotations I tried
didn't take right).
I don't have a good ARM setup at the moment to actually test these
changes. Can others test them? They seem to work for me on x86, but
that isn't saying much.
Warner
Index: kdump.c
===================================================================
--- kdump.c (revision 203976)
+++ kdump.c (working copy)
@@ -1328,6 +1328,8 @@
char *name, *data;
size_t namelen, datalen;
int i;
+ struct stat sb;
+ struct sockaddr_storage ss;
for (name = buf, namelen = 0;
namelen < buflen && name[namelen] != '\0';
@@ -1348,12 +1350,16 @@
if (strcmp(name, "stat") == 0) {
if (datalen != sizeof(struct stat))
goto invalid;
- ktrstat((struct stat *)data);
+ memcpy(&sb, data, datalen);
+ ktrstat(&sb);
} else if (strcmp(name, "sockaddr") == 0) {
+ if (datalen > sizeof(ss))
+ goto invalid;
+ memcpy(&ss, data, datalen);
if (datalen < sizeof(struct sockaddr) ||
- datalen != ((struct sockaddr *)(data))->sa_len)
+ datalen != ss.ss_len)
goto invalid;
- ktrsockaddr((struct sockaddr *)data);
+ ktrsockaddr((struct sockaddr *)&ss);
} else {
printf("unknown structure\n");
}
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100217.100004.321689434032786752.imp>