On Mon, 2009-06-15 at 22:18 +0200, Heiko Carstens wrote: > On Mon, Jun 15, 2009 at 11:35:56PM +0200, Roel Kluin wrote: > > Ensure that q_nr is within the bounds of array irq_ptr->input_qs[] and > > ->output_qs[]. > > > > Signed-off-by: Roel Kluin <roel.kluin@xxxxxxxxx> > > --- > > I didn't notice this when writing my previous patch. Should bufnr and > > count be larger than 0 as well? > > > > diff --git a/drivers/s390/cio/qdio_main.c b/drivers/s390/cio/qdio_main.c > > index d79cf5b..9dbc4b1 100644 > > --- a/drivers/s390/cio/qdio_main.c > > +++ b/drivers/s390/cio/qdio_main.c > > @@ -1494,7 +1494,7 @@ int do_QDIO(struct ccw_device *cdev, unsigned int callflags, > > > > if ((bufnr > QDIO_MAX_BUFFERS_PER_Q) || > > (count > QDIO_MAX_BUFFERS_PER_Q) || > > - (q_nr >= QDIO_MAX_QUEUES_PER_IRQ)) > > + (q_nr >= QDIO_MAX_QUEUES_PER_IRQ) || (q_nr < 0)) > > return -EINVAL; > > These are just sanity checks. I'd rather prefer to see all of them go away or > have them under an #ifdef DEBUG. > But it's Jan's call. He is the maintainer of the driver. Well, I'd like to kill that check. This is in the hot path and the q_nr check is useless since it only checks for the theoretical limit, in reality the q_nr is 1 or 4, not 32. But catching that is overkill and specifying the wrong queue would simply lead to a NULL pointer later, which should be easy enough to find. To ensure bufnr and count are > 0 we could use unsigned int. And the bufnr check was also incorrect. How about that: Signed-off-by: Jan Glauber <jang@xxxxxxxxxxxxxxxxxx> --- arch/s390/include/asm/qdio.h | 2 +- drivers/s390/cio/qdio_main.c | 9 ++------- 2 files changed, 3 insertions(+), 8 deletions(-) Index: linux-2.5/drivers/s390/cio/qdio_main.c =================================================================== --- linux-2.5.orig/drivers/s390/cio/qdio_main.c +++ linux-2.5/drivers/s390/cio/qdio_main.c @@ -1497,18 +1497,13 @@ out: * @count: how many buffers to process */ int do_QDIO(struct ccw_device *cdev, unsigned int callflags, - int q_nr, int bufnr, int count) + int q_nr, unsigned int bufnr, unsigned int count) { struct qdio_irq *irq_ptr; - if ((bufnr > QDIO_MAX_BUFFERS_PER_Q) || - (count > QDIO_MAX_BUFFERS_PER_Q) || - (q_nr >= QDIO_MAX_QUEUES_PER_IRQ)) + if (bufnr >= QDIO_MAX_BUFFERS_PER_Q || count > QDIO_MAX_BUFFES_PER_Q) return -EINVAL; - if (!count) - return 0; - irq_ptr = cdev->private->qdio_data; if (!irq_ptr) return -ENODEV; Index: linux-2.5/arch/s390/include/asm/qdio.h =================================================================== --- linux-2.5.orig/arch/s390/include/asm/qdio.h +++ linux-2.5/arch/s390/include/asm/qdio.h @@ -380,7 +380,7 @@ extern int qdio_establish(struct qdio_in extern int qdio_activate(struct ccw_device *); extern int do_QDIO(struct ccw_device *cdev, unsigned int callflags, - int q_nr, int bufnr, int count); + int q_nr, unsigned int bufnr, unsigned int count); extern int qdio_cleanup(struct ccw_device*, int); extern int qdio_shutdown(struct ccw_device*, int); extern int qdio_free(struct ccw_device *); -- To unsubscribe from this list: send the line "unsubscribe linux-s390" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html