[PATCH 1/2] staging: lustre: lov: fix dereference of ERR_PTR

[PATCH 1/2] staging: lustre: lov: fix dereference of ERR_PTR

[Sudip Mukherjee]

If lov_sub_get() fails then it returns the error code in ERR_PTR, but
here we were dereferencing sub without checking if lov_sub_get() has
actually succeeded or not. And on error we can directly return the error
code from lov_io_fault_start() as it return 0 on success and the error
code on error.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/lustre/lustre/lov/lov_io.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/lustre/lustre/lov/lov_io.c b/drivers/staging/lustre/lustre/lov/lov_io.c
index bf36291..b5b2580 100644
--- a/drivers/staging/lustre/lustre/lov/lov_io.c
+++ b/drivers/staging/lustre/lustre/lov/lov_io.c
@@ -729,6 +729,8 @@ static int lov_io_fault_start(const struct lu_env *env,
 	fio = &ios->cis_io->u.ci_fault;
 	lio = cl2lov_io(env, ios);
 	sub = lov_sub_get(env, lio, lov_page_stripe(fio->ft_page));
+	if (IS_ERR(sub))
+		return PTR_ERR(sub);
 	sub->sub_io->u.ci_fault.ft_nob = fio->ft_nob;
 	lov_sub_put(sub);
 	return lov_io_start(env, ios);
-- 
1.9.1

[PATCH 2/2] staging: lustre: lov: remove always false condition

[Sudip Mukherjee]

The member qc_idx of struct if_quotactl is unsigned and hence it can
never be less than zero.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/lustre/lustre/lov/lov_obd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/lustre/lustre/lov/lov_obd.c b/drivers/staging/lustre/lustre/lov/lov_obd.c
index 2a2fd8d..906503b 100644
--- a/drivers/staging/lustre/lustre/lov/lov_obd.c
+++ b/drivers/staging/lustre/lustre/lov/lov_obd.c
@@ -1487,7 +1487,7 @@ static int lov_iocontrol(unsigned int cmd, struct obd_export *exp, int len,
 		struct obd_quotactl *oqctl;
 
 		if (qctl->qc_valid == QC_OSTIDX) {
-			if (qctl->qc_idx < 0 || count <= qctl->qc_idx)
+			if (count <= qctl->qc_idx)
 				return -EINVAL;
 
 			tgt = lov->lov_tgts[qctl->qc_idx];
-- 
1.9.1

Re: [PATCH 2/2] staging: lustre: lov: remove always false condition

[Joe Perches]

On Tue, 2015-09-08 at 21:53 +0530, Sudip Mukherjee wrote:
> The member qc_idx of struct if_quotactl is unsigned and hence it can
> never be less than zero.
[]
> diff --git a/drivers/staging/lustre/lustre/lov/lov_obd.c b/drivers/staging/lustre/lustre/lov/lov_obd.c
[]
> @@ -1487,7 +1487,7 @@ static int lov_iocontrol(unsigned int cmd, struct obd_export *exp, int len,
>  		struct obd_quotactl *oqctl;
>  
>  		if (qctl->qc_valid == QC_OSTIDX) {
> -			if (qctl->qc_idx < 0 || count <= qctl->qc_idx)
> +			if (count <= qctl->qc_idx)

Perhaps this test would be clearer reversed too

			if (qctl->qc_idx >= count)

>  				return -EINVAL;

Re: [PATCH 2/2] staging: lustre: lov: remove always false condition

[Sudip Mukherjee]

On Tue, Sep 08, 2015 at 09:53:09AM -0700, Joe Perches wrote:
> On Tue, 2015-09-08 at 21:53 +0530, Sudip Mukherjee wrote:
> >  		if (qctl->qc_valid == QC_OSTIDX) {
> > -			if (qctl->qc_idx < 0 || count <= qctl->qc_idx)
> > +			if (count <= qctl->qc_idx)
> 
> Perhaps this test would be clearer reversed too
> 
> 			if (qctl->qc_idx >= count)
There are more such comparison in that file, like:
1) if (ost_idx >= lov->desc.ld_tgt_count)
2) if (index >= lov->lov_tgt_size)
and some more.
And if I include it in this patch then it will become separate change.
I will better send a separate patch for this.

Regards
Sudip