| Message ID | 20240801122742.5001-1-carlos.lopezr4096@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | lib: sbi: dbtr: fix potential NULL pointer dereferences | expand |
On Thu, Aug 1, 2024 at 6:03 PM Carlos López <carlos.lopezr4096@gmail.com> wrote: > > In several dbtr functions, we first check that the dbtr trigger is not > NULL and that its state is what we expect. However, it only makes > sense to perform the second check if the dbtr trigger is not NULL. > Othwerwise we will dereference a NULL pointer. Thus, change the > condition so that it shortcuts to the first check if necessary. > > Signed-off-by: Carlos López <carlos.lopezr4096@gmail.com> LGTM. Reviewed-by: Anup Patel <anup@brainfault.org> Thanks, Anup > --- > lib/sbi/sbi_dbtr.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/lib/sbi/sbi_dbtr.c b/lib/sbi/sbi_dbtr.c > index 6e2083e..27a8b91 100644 > --- a/lib/sbi/sbi_dbtr.c > +++ b/lib/sbi/sbi_dbtr.c > @@ -357,7 +357,7 @@ static void dbtr_trigger_enable(struct sbi_dbtr_trigger *trig) > unsigned long state; > unsigned long tdata1; > > - if (!trig && !(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED))) > + if (!trig || !(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED))) > return; > > state = trig->state; > @@ -403,7 +403,7 @@ static void dbtr_trigger_disable(struct sbi_dbtr_trigger *trig) > { > unsigned long tdata1; > > - if (!trig && !(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED))) > + if (!trig || !(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED))) > return; > > tdata1 = trig->tdata1; > @@ -429,7 +429,7 @@ static void dbtr_trigger_disable(struct sbi_dbtr_trigger *trig) > > static void dbtr_trigger_clear(struct sbi_dbtr_trigger *trig) > { > - if (!trig && !(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED))) > + if (!trig || !(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED))) > return; > > csr_write(CSR_TSELECT, trig->index); > -- > 2.39.2 > > > -- > opensbi mailing list > opensbi@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/opensbi
On Thu, Aug 1, 2024 at 6:03 PM Carlos López <carlos.lopezr4096@gmail.com> wrote: > > In several dbtr functions, we first check that the dbtr trigger is not > NULL and that its state is what we expect. However, it only makes > sense to perform the second check if the dbtr trigger is not NULL. > Othwerwise we will dereference a NULL pointer. Thus, change the > condition so that it shortcuts to the first check if necessary. > > Signed-off-by: Carlos López <carlos.lopezr4096@gmail.com> Applied this patch to the riscv/opensbi repo. Thanks, Anup > --- > lib/sbi/sbi_dbtr.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/lib/sbi/sbi_dbtr.c b/lib/sbi/sbi_dbtr.c > index 6e2083e..27a8b91 100644 > --- a/lib/sbi/sbi_dbtr.c > +++ b/lib/sbi/sbi_dbtr.c > @@ -357,7 +357,7 @@ static void dbtr_trigger_enable(struct sbi_dbtr_trigger *trig) > unsigned long state; > unsigned long tdata1; > > - if (!trig && !(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED))) > + if (!trig || !(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED))) > return; > > state = trig->state; > @@ -403,7 +403,7 @@ static void dbtr_trigger_disable(struct sbi_dbtr_trigger *trig) > { > unsigned long tdata1; > > - if (!trig && !(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED))) > + if (!trig || !(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED))) > return; > > tdata1 = trig->tdata1; > @@ -429,7 +429,7 @@ static void dbtr_trigger_disable(struct sbi_dbtr_trigger *trig) > > static void dbtr_trigger_clear(struct sbi_dbtr_trigger *trig) > { > - if (!trig && !(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED))) > + if (!trig || !(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED))) > return; > > csr_write(CSR_TSELECT, trig->index); > -- > 2.39.2 > > > -- > opensbi mailing list > opensbi@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/opensbi
diff --git a/lib/sbi/sbi_dbtr.c b/lib/sbi/sbi_dbtr.c index 6e2083e..27a8b91 100644 --- a/lib/sbi/sbi_dbtr.c +++ b/lib/sbi/sbi_dbtr.c @@ -357,7 +357,7 @@ static void dbtr_trigger_enable(struct sbi_dbtr_trigger *trig) unsigned long state; unsigned long tdata1; - if (!trig && !(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED))) + if (!trig || !(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED))) return; state = trig->state; @@ -403,7 +403,7 @@ static void dbtr_trigger_disable(struct sbi_dbtr_trigger *trig) { unsigned long tdata1; - if (!trig && !(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED))) + if (!trig || !(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED))) return; tdata1 = trig->tdata1; @@ -429,7 +429,7 @@ static void dbtr_trigger_disable(struct sbi_dbtr_trigger *trig) static void dbtr_trigger_clear(struct sbi_dbtr_trigger *trig) { - if (!trig && !(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED))) + if (!trig || !(trig->state & RV_DBTR_BIT_MASK(TS, MAPPED))) return; csr_write(CSR_TSELECT, trig->index);
In several dbtr functions, we first check that the dbtr trigger is not NULL and that its state is what we expect. However, it only makes sense to perform the second check if the dbtr trigger is not NULL. Othwerwise we will dereference a NULL pointer. Thus, change the condition so that it shortcuts to the first check if necessary. Signed-off-by: Carlos López <carlos.lopezr4096@gmail.com> --- lib/sbi/sbi_dbtr.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)