Subject: Re: kern/21474
To: None <peter@NetBSD.org, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,>
From: Peter Postma <peter@pointless.nl>
List: netbsd-bugs
Date: 10/16/2006 10:00:05
The following reply was made to PR kern/21474; it has been noted by GNATS.
From: Peter Postma <peter@pointless.nl>
To: gnats-bugs@NetBSD.org
Cc: martin@NetBSD.org, tron@NetBSD.org
Subject: Re: kern/21474
Date: Mon, 16 Oct 2006 11:58:22 +0200
--fUYQa+Pmc3FrFX/N
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
When the kernel crashes, the MTU on the interface is 0. ALTQ uses the MTU
to calculate some things and then divides by zero in rmc_wrr_set_weights():
> ifd->M_[i] = ifd->alloc_[i] /
> (ifd->num_[i] * ifd->maxpkt_);
ifd->maxpkt_ is set to the interface its MTU in rmc_init().
I think that the fix would be to disallow a MTU < 1. The attached patch
does that.
--
Peter Postma
--fUYQa+Pmc3FrFX/N
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="altq.diff"
Index: altq_rmclass.c
===================================================================
RCS file: /cvsroot/src/sys/altq/altq_rmclass.c,v
retrieving revision 1.15
diff -u -r1.15 altq_rmclass.c
--- altq_rmclass.c 12 Oct 2006 19:59:08 -0000 1.15
+++ altq_rmclass.c 16 Oct 2006 09:52:55 -0000
@@ -642,7 +642,7 @@
/*
- * void
+ * int
* rmc_init(...) - Initialize the resource management data structures
* associated with the output portion of interface 'ifp'. 'ifd' is
* where the structures will be built (for backwards compatibility, the
@@ -654,23 +654,29 @@
* is the maximum number of packets that the resource management
* code will allow to be queued 'downstream' (this is typically 1).
*
- * Returns: NONE
+ * Returns: 0 on success
*/
-void
+int
rmc_init(struct ifaltq *ifq, struct rm_ifdat *ifd, u_int nsecPerByte,
void (*restart)(struct ifaltq *), int maxq, int maxqueued, u_int maxidle,
int minidle, u_int offtime, int flags)
{
- int i, mtu;
+ int i, mtu;
/*
* Initialize the CBQ tracing/debug facility.
*/
CBQTRACEINIT();
- (void)memset((char *)ifd, 0, sizeof (*ifd));
mtu = ifq->altq_ifp->if_mtu;
+ if (mtu < 1) {
+ printf("altq: %s: invalid MTU (interface not initialized?)\n",
+ ifq->altq_ifp->if_xname);
+ return (EINVAL);
+ }
+
+ (void)memset((char *)ifd, 0, sizeof (*ifd));
ifd->ifq_ = ifq;
ifd->restart = restart;
ifd->maxqueued_ = maxqueued;
@@ -718,9 +724,11 @@
maxidle, minidle, offtime,
0, 0)) == NULL) {
printf("rmc_init: root class not allocated\n");
- return ;
+ return (ENOMEM);
}
ifd->root_->depth_ = 0;
+
+ return (0);
}
/*
Index: altq_rmclass.h
===================================================================
RCS file: /cvsroot/src/sys/altq/altq_rmclass.h,v
retrieving revision 1.7
diff -u -r1.7 altq_rmclass.h
--- altq_rmclass.h 12 Oct 2006 19:59:08 -0000 1.7
+++ altq_rmclass.h 16 Oct 2006 09:52:55 -0000
@@ -248,7 +248,7 @@
extern void rmc_delete_class(struct rm_ifdat *, struct rm_class *);
extern int rmc_modclass(struct rm_class *, u_int, int,
u_int, int, u_int, int);
-extern void rmc_init(struct ifaltq *, struct rm_ifdat *, u_int,
+extern int rmc_init(struct ifaltq *, struct rm_ifdat *, u_int,
void (*)(struct ifaltq *),
int, int, u_int, int, u_int, int);
extern int rmc_queue_packet(struct rm_class *, mbuf_t *);
Index: altq_cbq.c
===================================================================
RCS file: /cvsroot/src/sys/altq/altq_cbq.c,v
retrieving revision 1.20
diff -u -r1.20 altq_cbq.c
--- altq_cbq.c 13 Oct 2006 09:57:28 -0000 1.20
+++ altq_cbq.c 16 Oct 2006 09:52:56 -0000
@@ -314,7 +314,7 @@
cbq_state_t *cbqp;
struct rm_class *cl;
struct cbq_opts *opts;
- int i;
+ int i, error;
if ((cbqp = a->altq_disc) == NULL)
return (EINVAL);
@@ -389,10 +389,12 @@
* interface.
*/
if ((opts->flags & CBQCLF_CLASSMASK) == CBQCLF_ROOTCLASS) {
- rmc_init(cbqp->ifnp.ifq_, &cbqp->ifnp, opts->ns_per_byte,
- cbqrestart, a->qlimit, RM_MAXQUEUED,
+ error = rmc_init(cbqp->ifnp.ifq_, &cbqp->ifnp,
+ opts->ns_per_byte, cbqrestart, a->qlimit, RM_MAXQUEUED,
opts->maxidle, opts->minidle, opts->offtime,
opts->flags);
+ if (error != 0)
+ return (error);
cl = cbqp->ifnp.root_;
} else {
cl = rmc_newclass(a->priority,
@@ -704,7 +706,7 @@
struct rm_class *cl;
cbq_class_spec_t *spec = &acp->cbq_class;
u_int32_t chandle;
- int i;
+ int i, error;
/*
* allocate class handle
@@ -721,10 +723,12 @@
* interface.
*/
if ((spec->flags & CBQCLF_CLASSMASK) == CBQCLF_ROOTCLASS) {
- rmc_init(cbqp->ifnp.ifq_, &cbqp->ifnp, spec->nano_sec_per_byte,
- cbqrestart, spec->maxq, RM_MAXQUEUED,
- spec->maxidle, spec->minidle, spec->offtime,
- spec->flags);
+ error = rmc_init(cbqp->ifnp.ifq_, &cbqp->ifnp,
+ spec->nano_sec_per_byte, cbqrestart, spec->maxq,
+ RM_MAXQUEUED, spec->maxidle, spec->minidle, spec->offtime,
+ spec->flags);
+ if (error)
+ return (error);
cl = cbqp->ifnp.root_;
} else {
cl = rmc_newclass(spec->priority,
--fUYQa+Pmc3FrFX/N--