| Message ID | 20240809135346.978320-1-haris.iqbal@ionos.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [for-next] block/rnbd-srv: Add sanity check and remove redundant assignment | expand |
On Fri, Aug 9, 2024 at 3:54 PM Md Haris Iqbal <haris.iqbal@ionos.com> wrote: > > The bio->bi_iter.bi_size is updated when bio_add_page() is called. So we > do not need to assign msg->bi_size again to it, since its redudant and > can also be harmful. Instead we can use it to add a sanity check, which > checks the locally calculated bi_size, with the one sent in msg. > > Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com> > Signed-off-by: Jack Wang <jinpu.wang@ionos.com> > Signed-off-by: Grzegorz Prajsner <grzegorz.prajsner@ionos.com> > --- > drivers/block/rnbd/rnbd-srv.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c > index f6e3a3c4b76c..08ce6d96d04c 100644 > --- a/drivers/block/rnbd/rnbd-srv.c > +++ b/drivers/block/rnbd/rnbd-srv.c > @@ -149,15 +149,22 @@ static int process_rdma(struct rnbd_srv_session *srv_sess, > rnbd_to_bio_flags(le32_to_cpu(msg->rw)), GFP_KERNEL); > if (bio_add_page(bio, virt_to_page(data), datalen, > offset_in_page(data)) != datalen) { > - rnbd_srv_err(sess_dev, "Failed to map data to bio\n"); > + rnbd_srv_err_rl(sess_dev, "Failed to map data to bio\n"); > err = -EINVAL; > goto bio_put; > } > > + bio->bi_opf = rnbd_to_bio_flags(le32_to_cpu(msg->rw)); > + if (bio_has_data(bio) && > + bio->bi_iter.bi_size != le32_to_cpu(msg->bi_size)) { > + rnbd_srv_err_rl(sess_dev, "Datalen mismatch: bio bi_size (%u), bi_size (%u)\n", > + bio->bi_iter.bi_size, msg->bi_size); > + err = -EINVAL; > + goto bio_put; > + } > bio->bi_end_io = rnbd_dev_bi_end_io; > bio->bi_private = priv; > bio->bi_iter.bi_sector = le64_to_cpu(msg->sector); > - bio->bi_iter.bi_size = le32_to_cpu(msg->bi_size); > prio = srv_sess->ver < RNBD_PROTO_VER_MAJOR || > usrlen < sizeof(*msg) ? 0 : le16_to_cpu(msg->prio); > bio_set_prio(bio, prio); > -- > 2.25.1 Gentle ping. >
On Wed, Aug 21, 2024 at 1:25 PM Haris Iqbal <haris.iqbal@ionos.com> wrote: > > On Fri, Aug 9, 2024 at 3:54 PM Md Haris Iqbal <haris.iqbal@ionos.com> wrote: > > > > The bio->bi_iter.bi_size is updated when bio_add_page() is called. So we > > do not need to assign msg->bi_size again to it, since its redudant and > > can also be harmful. Instead we can use it to add a sanity check, which > > checks the locally calculated bi_size, with the one sent in msg. > > > > Signed-off-by: Md Haris Iqbal <haris.iqbal@ionos.com> > > Signed-off-by: Jack Wang <jinpu.wang@ionos.com> > > Signed-off-by: Grzegorz Prajsner <grzegorz.prajsner@ionos.com> > > --- > > drivers/block/rnbd/rnbd-srv.c | 11 +++++++++-- > > 1 file changed, 9 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c > > index f6e3a3c4b76c..08ce6d96d04c 100644 > > --- a/drivers/block/rnbd/rnbd-srv.c > > +++ b/drivers/block/rnbd/rnbd-srv.c > > @@ -149,15 +149,22 @@ static int process_rdma(struct rnbd_srv_session *srv_sess, > > rnbd_to_bio_flags(le32_to_cpu(msg->rw)), GFP_KERNEL); > > if (bio_add_page(bio, virt_to_page(data), datalen, > > offset_in_page(data)) != datalen) { > > - rnbd_srv_err(sess_dev, "Failed to map data to bio\n"); > > + rnbd_srv_err_rl(sess_dev, "Failed to map data to bio\n"); > > err = -EINVAL; > > goto bio_put; > > } > > > > + bio->bi_opf = rnbd_to_bio_flags(le32_to_cpu(msg->rw)); > > + if (bio_has_data(bio) && > > + bio->bi_iter.bi_size != le32_to_cpu(msg->bi_size)) { > > + rnbd_srv_err_rl(sess_dev, "Datalen mismatch: bio bi_size (%u), bi_size (%u)\n", > > + bio->bi_iter.bi_size, msg->bi_size); > > + err = -EINVAL; > > + goto bio_put; > > + } > > bio->bi_end_io = rnbd_dev_bi_end_io; > > bio->bi_private = priv; > > bio->bi_iter.bi_sector = le64_to_cpu(msg->sector); > > - bio->bi_iter.bi_size = le32_to_cpu(msg->bi_size); > > prio = srv_sess->ver < RNBD_PROTO_VER_MAJOR || > > usrlen < sizeof(*msg) ? 0 : le16_to_cpu(msg->prio); > > bio_set_prio(bio, prio); > > -- > > 2.25.1 > > Gentle ping. Hi Jens, Could you please pink up this patch if there are no comments. Thanks > > >
On Fri, 09 Aug 2024 15:53:46 +0200, Md Haris Iqbal wrote: > The bio->bi_iter.bi_size is updated when bio_add_page() is called. So we > do not need to assign msg->bi_size again to it, since its redudant and > can also be harmful. Instead we can use it to add a sanity check, which > checks the locally calculated bi_size, with the one sent in msg. > > Applied, thanks! [1/1] block/rnbd-srv: Add sanity check and remove redundant assignment commit: f6f84be089c9d6f5e3e1228c389e51c7ae7bad1a Best regards,
diff --git a/drivers/block/rnbd/rnbd-srv.c b/drivers/block/rnbd/rnbd-srv.c index f6e3a3c4b76c..08ce6d96d04c 100644 --- a/drivers/block/rnbd/rnbd-srv.c +++ b/drivers/block/rnbd/rnbd-srv.c @@ -149,15 +149,22 @@ static int process_rdma(struct rnbd_srv_session *srv_sess, rnbd_to_bio_flags(le32_to_cpu(msg->rw)), GFP_KERNEL); if (bio_add_page(bio, virt_to_page(data), datalen, offset_in_page(data)) != datalen) { - rnbd_srv_err(sess_dev, "Failed to map data to bio\n"); + rnbd_srv_err_rl(sess_dev, "Failed to map data to bio\n"); err = -EINVAL; goto bio_put; } + bio->bi_opf = rnbd_to_bio_flags(le32_to_cpu(msg->rw)); + if (bio_has_data(bio) && + bio->bi_iter.bi_size != le32_to_cpu(msg->bi_size)) { + rnbd_srv_err_rl(sess_dev, "Datalen mismatch: bio bi_size (%u), bi_size (%u)\n", + bio->bi_iter.bi_size, msg->bi_size); + err = -EINVAL; + goto bio_put; + } bio->bi_end_io = rnbd_dev_bi_end_io; bio->bi_private = priv; bio->bi_iter.bi_sector = le64_to_cpu(msg->sector); - bio->bi_iter.bi_size = le32_to_cpu(msg->bi_size); prio = srv_sess->ver < RNBD_PROTO_VER_MAJOR || usrlen < sizeof(*msg) ? 0 : le16_to_cpu(msg->prio); bio_set_prio(bio, prio);