IPSEC/crypto is broken in FreeBSD/powerpc 7.0-RELEASE!
John Baldwin
jhb at FreeBSD.org
Mon Mar 10 15:02:49 PDT 2008
On Tuesday 04 March 2008 12:13:46 am Sam Leffler wrote:
> Marcel Moolenaar wrote:
> >
> > On Mar 3, 2008, at 6:18 PM, Maxim Sobolev wrote:
> >
> >> Hi,
> >>
> >> It appears to be that "options IPSEC" along with "device crypto"
> >> breaks FreeBSD/powerpc kernel badly. When enabling these options,
> >> apparently kernel doesn't perform any initialization tasks (I don't
> >> see usual probe/init sequence output) but jumps straight into root fs
> >> mounting after initing crypto(4) and ipsec(4), which is not usable
> >> since no devices has been attached. Keyboard is not working either.
> >
> > The problem is with device crypto. It attaches to nexus(4) and
> > expects to be the only child. As you can see from the log, all
> > children of nexus suddenly become instantiations of cryptosoft(4)
> > rather then the usual drivers that attach.
> >
> > The swcr_probe() function should check that the device it gets
> > is really the one created for it.
> >
>
> Don't know about "expects to be the only child" but I did was jhb said
> was right. If you know otherwise please fix it.
My mistake was assuming that all platforms treated nexus(4) the same in that
each child of nexus has a fixed devclass when it is created. BTW, it looks
like the common crypto code doesn't really need a full on device_t, it really
just wants a 'kobj' that uses the class and a name of of some sort. Other
places in the kernel use a plain old switch table (e.g. struct ifnet and
struct cdev with cdevsw) for such things since kobj has some additional
overhead. Is there any reason the crypto stuff can't use a separate object
to present a crypto device that is separate from the device_t? The least
code churn might be to stay with kobj. Hmm, you could actually reuse the
device_t as the kobj for hardware devices. All the MI crypto code needs is a
name, so you would split out the name instead of using device_get_nameunit().
Something like this would be a start. (The hardware drivers would simply
pass device_get_nameunit(dev) as the second arg to crypto_get_driverid()).
The one routine needs to be changed to return the crypto device name
(cc_name) in a caller-supplied buffer rather than the device_t, but other
than those changes (and updating function prototypes in headers) this should
be most of what you would need. It would of course be cleaner to not reuse
the device_t and have cryptocaps be a bit stronger of an object (maybe it
becomes a kobj and crypto_get_driverid() takes a class pointer rather than a
kobj).
Index: opencrypto/crypto.c
===================================================================
RCS file: /usr/cvs/src/sys/opencrypto/crypto.c,v
retrieving revision 1.28
diff -u -r1.28 crypto.c
--- opencrypto/crypto.c 20 Oct 2007 23:23:22 -0000 1.28
+++ opencrypto/crypto.c 10 Mar 2008 21:05:03 -0000
@@ -99,7 +99,8 @@
* Not tagged fields are read-only.
*/
struct cryptocap {
- device_t cc_dev; /* (d) device/driver */
+ struct kobj *cc_dev; /* (d) device/driver */
+ const char *cc_name; /* (d) device name */
u_int32_t cc_sessions; /* (d) # of sessions */
u_int32_t cc_koperations; /* (d) # os asym operations */
/*
@@ -476,7 +477,7 @@
* support for the algorithms they handle.
*/
int32_t
-crypto_get_driverid(device_t dev, int flags)
+crypto_get_driverid(struct kobj *dev, const char *name, int flags)
{
struct cryptocap *newdrv;
int i;
@@ -538,7 +539,7 @@
/*
* Lookup a driver by name. We match against the full device
* name and unit, and against just the name. The latter gives
- * us a simple widlcarding by device name. On success return the
+ * us a simple wildcarding by device name. On success return the
* driver/hardware identifier; otherwise return -1.
*/
int
@@ -548,12 +549,17 @@
CRYPTO_DRIVER_LOCK();
for (i = 0; i < crypto_drivers_num; i++) {
- device_t dev = crypto_drivers[i].cc_dev;
- if (dev == NULL ||
+ if (crypto_drivers[i].cc_dev == NULL ||
(crypto_drivers[i].cc_flags & CRYPTOCAP_F_CLEANUP))
continue;
- if (strncmp(match, device_get_nameunit(dev), len) == 0 ||
- strncmp(match, device_get_name(dev), len) == 0)
+ /*
+ * Previously this did two checks. However, the second
+ * check was never relevant. If the user asked for just
+ * "foo" then 'len' will be 3 and the 'strncmp()' will
+ * match "foo0" just fine without requiring a separate
+ * match against "foo".
+ */
+ if (strncmp(match, cc->cc_name, len) == 0)
break;
}
CRYPTO_DRIVER_UNLOCK();
@@ -563,6 +569,8 @@
/*
* Return the device_t for the specified driver or NULL
* if the driver identifier is invalid.
+ *
+ * XXX: This needs to change to copy the name into a caller-supplied buffer.
*/
device_t
crypto_find_device_byhid(int hid)
@@ -605,7 +613,7 @@
cap->cc_kalg[kalg] = flags | CRYPTO_ALG_FLAG_SUPPORTED;
if (bootverbose)
printf("crypto: %s registers key alg %u flags %u\n"
- , device_get_nameunit(cap->cc_dev)
+ , cap->cc_name
, kalg
, flags
);
@@ -644,7 +652,7 @@
cap->cc_max_op_len[alg] = maxoplen;
if (bootverbose)
printf("crypto: %s registers alg %u flags %u maxoplen %u\n"
- , device_get_nameunit(cap->cc_dev)
+ , cap->cc_name
, alg
, flags
, maxoplen
@@ -1454,7 +1462,7 @@
if (cap->cc_dev == NULL)
continue;
db_printf("%-12s %4u %4u %08x %2u %2u\n"
- , device_get_nameunit(cap->cc_dev)
+ , cap->cc_name
, cap->cc_sessions
, cap->cc_koperations
, cap->cc_flags
Index: opencrypto/cryptosoft.c
===================================================================
RCS file: /usr/cvs/src/sys/opencrypto/cryptosoft.c,v
retrieving revision 1.19
diff -u -r1.19 cryptosoft.c
--- opencrypto/cryptosoft.c 9 May 2007 19:37:02 -0000 1.19
+++ opencrypto/cryptosoft.c 10 Mar 2008 21:22:08 -0000
@@ -61,7 +61,7 @@
static int swcr_encdec(struct cryptodesc *, struct swcr_data *, caddr_t,
int);
static int swcr_authcompute(struct cryptodesc *, struct swcr_data *, caddr_t,
int);
static int swcr_compdec(struct cryptodesc *, struct swcr_data *, caddr_t,
int);
-static int swcr_freesession(device_t dev, u_int64_t tid);
+static int swcr_freesession(struct kobj *kobj, u_int64_t tid);
/*
* Apply a symmetric encryption/decryption algorithm.
@@ -584,7 +584,7 @@
* Generate a new software session.
*/
static int
-swcr_newsession(device_t dev, u_int32_t *sid, struct cryptoini *cri)
+swcr_newsession(struct kobj *kobj, u_int32_t *sid, struct cryptoini *cri)
{
struct swcr_data **swd;
struct auth_hash *axf;
@@ -793,7 +793,7 @@
* Free a session.
*/
static int
-swcr_freesession(device_t dev, u_int64_t tid)
+swcr_freesession(struct kobj *kobj, u_int64_t tid)
{
struct swcr_data *swd;
struct enc_xform *txf;
@@ -882,7 +882,7 @@
* Process a software request.
*/
static int
-swcr_process(device_t dev, struct cryptop *crp, int hint)
+swcr_process(struct kobj *kobj, struct cryptop *crp, int hint)
{
struct cryptodesc *crd;
struct swcr_data *sw;
@@ -976,33 +976,33 @@
return 0;
}
-static void
-swcr_identify(device_t *dev, device_t parent)
-{
- /* NB: order 10 is so we get attached after h/w devices */
- if (device_find_child(parent, "cryptosoft", -1) == NULL &&
- BUS_ADD_CHILD(parent, 10, "cryptosoft", -1) == 0)
- panic("cryptosoft: could not attach");
-}
+static struct kobj_method swcr_methods[] = {
+ KOBJMETHOD(cryptodev_newsession, swcr_newsession),
+ KOBJMETHOD(cryptodev_freesession, swcr_freesession),
+ KOBJMETHOD(cryptodev_process, swcr_process),
-static int
-swcr_probe(device_t dev)
-{
- device_set_desc(dev, "software crypto");
- return (0);
-}
+ {0, 0},
+};
-static int
-swcr_attach(device_t dev)
+DEFINE_CLASS(cryptosoft, swcr_methods, 0);
+
+static struct kobj *swcr_kobj;
+
+static void
+swcr_init(void)
{
+
+ kobj_class_compile(&cryptosoft_class);
+ kobj_init(&swcr_kobj, &cryptosoft_class);
+ swcr_kobj = kobj_create(&cryptosoft_class, M_CRYPTO_DATA, M_WAITOK);
memset(hmac_ipad_buffer, HMAC_IPAD_VAL, HMAC_MAX_BLOCK_LEN);
memset(hmac_opad_buffer, HMAC_OPAD_VAL, HMAC_MAX_BLOCK_LEN);
- swcr_id = crypto_get_driverid(dev,
+ swcr_id = crypto_get_driverid(swcr_kobj, "cryptosoft0",
CRYPTOCAP_F_SOFTWARE | CRYPTOCAP_F_SYNC);
if (swcr_id < 0) {
- device_printf(dev, "cannot initialize!");
- return ENOMEM;
+ printf("cryptosoft0: cannot initialize!");
+ return;
}
#define REGISTER(alg) \
crypto_register(swcr_id, alg, 0,0)
@@ -1027,38 +1027,21 @@
REGISTER(CRYPTO_CAMELLIA_CBC);
REGISTER(CRYPTO_DEFLATE_COMP);
#undef REGISTER
-
- return 0;
}
static void
-swcr_detach(device_t dev)
+swcr_destroy(void)
{
crypto_unregister_all(swcr_id);
if (swcr_sessions != NULL)
FREE(swcr_sessions, M_CRYPTO_DATA);
+ if (swcr_kobj != NULL) {
+ kobj_delete(swcr_kobj, M_CRYPTO_DATA);
+ swcr_kobj = NULL;
+ kobj_class_free(&cryptosoft_class);
+ }
}
-static device_method_t swcr_methods[] = {
- DEVMETHOD(device_identify, swcr_identify),
- DEVMETHOD(device_probe, swcr_probe),
- DEVMETHOD(device_attach, swcr_attach),
- DEVMETHOD(device_detach, swcr_detach),
-
- DEVMETHOD(cryptodev_newsession, swcr_newsession),
- DEVMETHOD(cryptodev_freesession,swcr_freesession),
- DEVMETHOD(cryptodev_process, swcr_process),
-
- {0, 0},
-};
-
-static driver_t swcr_driver = {
- "cryptosoft",
- swcr_methods,
- 0, /* NB: no softc */
-};
-static devclass_t swcr_devclass;
-
/*
* NB: We explicitly reference the crypto module so we
* get the necessary ordering when built as a loadable
@@ -1067,7 +1050,35 @@
* normal module dependencies would handle things).
*/
extern int crypto_modevent(struct module *, int, void *);
-/* XXX where to attach */
-DRIVER_MODULE(cryptosoft, nexus, swcr_driver, swcr_devclass,
crypto_modevent,0);
+
+static int
+swcr_modevent(struct module *module, int cmd, void *arg)
+{
+ int error;
+
+ switch (cmd) {
+ case MOD_LOAD:
+ error = crypto_modevent(module, cmd, arg);
+ if (error)
+ return (error);
+ swcr_init();
+ return (0);
+ case MOD_UNLOAD:
+ swcr_destroy();
+ return (crypto_modevent(module, cmd, arg));
+ case MOD_QUIESCE:
+ return (crypto_modevent(module, cmd, arg));
+ default:
+ return (EOPNOTSUPP);
+ }
+}
+
+static moduledata_t cryptosoft_module = {
+ "cryptosoft",
+ swcr_modevent,
+ 0
+};
+
+DECLARE_MODULE(cryptosoft, cryptosoft_module, SI_SUB_DRIVER, SI_ORDER_ANY);
MODULE_VERSION(cryptosoft, 1);
MODULE_DEPEND(cryptosoft, crypto, 1, 1, 1);
--
John Baldwin
More information about the freebsd-current
mailing list