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
