Return error code for failure input for sscanf in parse_cblock_range()

[PATCH] Return error code for failure input for sscanf in parse_cblock_range()

Posted by Dipendra Khadka 1 year, 4 months ago

Smatch reported following:
'''
drivers/md/dm-cache-target.c:3204 parse_cblock_range() warn: sscanf doesn't return error codes
drivers/md/dm-cache-target.c:3217 parse_cblock_range() warn: sscanf doesn't return error codes
'''

Since, the only negative value that is returned by sscanf is -1.
Returning -ENVAL when sscanf returns -1.

Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>
---
 drivers/md/dm-cache-target.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
index 17f0fab1e254..c35d65e310d6 100644
--- a/drivers/md/dm-cache-target.c
+++ b/drivers/md/dm-cache-target.c
@@ -3200,8 +3200,8 @@ static int parse_cblock_range(struct cache *cache, const char *str,
 	 * Try and parse form (ii) first.
 	 */
 	r = sscanf(str, "%llu-%llu%c", &b, &e, &dummy);
-	if (r < 0)
-		return r;
+	if (r == -1)
+		return -EINVAL;
 
 	if (r == 2) {
 		result->begin = to_cblock(b);
@@ -3213,8 +3213,8 @@ static int parse_cblock_range(struct cache *cache, const char *str,
 	 * That didn't work, try form (i).
 	 */
 	r = sscanf(str, "%llu%c", &b, &dummy);
-	if (r < 0)
-		return r;
+	if (r == -1)
+		return -EINVAL;
 
 	if (r == 1) {
 		result->begin = to_cblock(b);
-- 
2.43.0

Re: [PATCH] Return error code for failure input for sscanf in parse_cblock_range()

Posted by Mikulas Patocka 1 year, 4 months ago

Hi

I skimmed through the sscanf source code and it seems that it can't return 
a negative value at all.

I think, this should be:
	r = sscanf(str, "%llu-%llu%c", &b, &e, &dummy);
	if (r != 2)
		return -EINVAL;

Mikulas

On Sun, 22 Sep 2024, Dipendra Khadka wrote:

> Smatch reported following:
> '''
> drivers/md/dm-cache-target.c:3204 parse_cblock_range() warn: sscanf doesn't return error codes
> drivers/md/dm-cache-target.c:3217 parse_cblock_range() warn: sscanf doesn't return error codes
> '''
> 
> Since, the only negative value that is returned by sscanf is -1.
> Returning -ENVAL when sscanf returns -1.
> 
> Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>
> ---
>  drivers/md/dm-cache-target.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
> index 17f0fab1e254..c35d65e310d6 100644
> --- a/drivers/md/dm-cache-target.c
> +++ b/drivers/md/dm-cache-target.c
> @@ -3200,8 +3200,8 @@ static int parse_cblock_range(struct cache *cache, const char *str,
>  	 * Try and parse form (ii) first.
>  	 */
>  	r = sscanf(str, "%llu-%llu%c", &b, &e, &dummy);
> -	if (r < 0)
> -		return r;
> +	if (r == -1)
> +		return -EINVAL;
>  
>  	if (r == 2) {
>  		result->begin = to_cblock(b);
> @@ -3213,8 +3213,8 @@ static int parse_cblock_range(struct cache *cache, const char *str,
>  	 * That didn't work, try form (i).
>  	 */
>  	r = sscanf(str, "%llu%c", &b, &dummy);
> -	if (r < 0)
> -		return r;
> +	if (r == -1)
> +		return -EINVAL;
>  
>  	if (r == 1) {
>  		result->begin = to_cblock(b);
> -- 
> 2.43.0
>

Re: [PATCH] Return error code for failure input for sscanf in parse_cblock_range()

Posted by Ming-Hung Tsai 1 year, 4 months ago

On Mon, Sep 23, 2024 at 12:47 AM Dipendra Khadka <kdipendra88@gmail.com> wrote:
>
> Smatch reported following:
> '''
> drivers/md/dm-cache-target.c:3204 parse_cblock_range() warn: sscanf doesn't return error codes
> drivers/md/dm-cache-target.c:3217 parse_cblock_range() warn: sscanf doesn't return error codes
> '''
>
> Since, the only negative value that is returned by sscanf is -1.
> Returning -ENVAL when sscanf returns -1.
>
> Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>
> ---
>  drivers/md/dm-cache-target.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
> index 17f0fab1e254..c35d65e310d6 100644
> --- a/drivers/md/dm-cache-target.c
> +++ b/drivers/md/dm-cache-target.c
> @@ -3200,8 +3200,8 @@ static int parse_cblock_range(struct cache *cache, const char *str,
>          * Try and parse form (ii) first.
>          */
>         r = sscanf(str, "%llu-%llu%c", &b, &e, &dummy);
> -       if (r < 0)
> -               return r;
> +       if (r == -1)
> +               return -EINVAL;
>
>         if (r == 2) {
>                 result->begin = to_cblock(b);
> @@ -3213,8 +3213,8 @@ static int parse_cblock_range(struct cache *cache, const char *str,
>          * That didn't work, try form (i).
>          */
>         r = sscanf(str, "%llu%c", &b, &dummy);
> -       if (r < 0)
> -               return r;
> +       if (r == -1)
> +               return -EINVAL;
>
>         if (r == 1) {
>                 result->begin = to_cblock(b);


Could you please clarify how to reproduce unexpected results? From
what I observe, the kernel's sscanf doesn't return -1 on an empty
input. Even if a negative value other than -EINVAL is returned, it is
handled by the callers.

Hank

> --
> 2.43.0
>
>

Re: [PATCH] Return error code for failure input for sscanf in parse_cblock_range()

Posted by Mikulas Patocka 1 year, 4 months ago


On Mon, 23 Sep 2024, Ming-Hung Tsai wrote:

> On Mon, Sep 23, 2024 at 12:47 AM Dipendra Khadka <kdipendra88@gmail.com> wrote:
> >
> > Smatch reported following:
> > '''
> > drivers/md/dm-cache-target.c:3204 parse_cblock_range() warn: sscanf doesn't return error codes
> > drivers/md/dm-cache-target.c:3217 parse_cblock_range() warn: sscanf doesn't return error codes
> > '''
> >
> > Since, the only negative value that is returned by sscanf is -1.
> > Returning -ENVAL when sscanf returns -1.
> >
> > Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>
> > ---
> >  drivers/md/dm-cache-target.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
> > index 17f0fab1e254..c35d65e310d6 100644
> > --- a/drivers/md/dm-cache-target.c
> > +++ b/drivers/md/dm-cache-target.c
> > @@ -3200,8 +3200,8 @@ static int parse_cblock_range(struct cache *cache, const char *str,
> >          * Try and parse form (ii) first.
> >          */
> >         r = sscanf(str, "%llu-%llu%c", &b, &e, &dummy);
> > -       if (r < 0)
> > -               return r;
> > +       if (r == -1)
> > +               return -EINVAL;
> >
> >         if (r == 2) {
> >                 result->begin = to_cblock(b);
> > @@ -3213,8 +3213,8 @@ static int parse_cblock_range(struct cache *cache, const char *str,
> >          * That didn't work, try form (i).
> >          */
> >         r = sscanf(str, "%llu%c", &b, &dummy);
> > -       if (r < 0)
> > -               return r;
> > +       if (r == -1)
> > +               return -EINVAL;
> >
> >         if (r == 1) {
> >                 result->begin = to_cblock(b);
> 
> 
> Could you please clarify how to reproduce unexpected results? From
> what I observe, the kernel's sscanf doesn't return -1 on an empty
> input. Even if a negative value other than -EINVAL is returned, it is
> handled by the callers.
> 
> Hank

I applied the patch, but I deleted the conditions "if (r == -1) return 
-EINVAL;"

sscanf in the kernel doesn't return negative numbers.

Mikulas

Re: [PATCH] Return error code for failure input for sscanf in parse_cblock_range()

Posted by Dipendra Khadka 1 year, 4 months ago

Hi,

On Mon, 23 Sept 2024 at 20:55, Mikulas Patocka <mpatocka@redhat.com> wrote:
>
>
>
> On Mon, 23 Sep 2024, Ming-Hung Tsai wrote:
>
> > On Mon, Sep 23, 2024 at 12:47 AM Dipendra Khadka <kdipendra88@gmail.com> wrote:
> > >
> > > Smatch reported following:
> > > '''
> > > drivers/md/dm-cache-target.c:3204 parse_cblock_range() warn: sscanf doesn't return error codes
> > > drivers/md/dm-cache-target.c:3217 parse_cblock_range() warn: sscanf doesn't return error codes
> > > '''
> > >
> > > Since, the only negative value that is returned by sscanf is -1.
> > > Returning -ENVAL when sscanf returns -1.
> > >
> > > Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>
> > > ---
> > >  drivers/md/dm-cache-target.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
> > > index 17f0fab1e254..c35d65e310d6 100644
> > > --- a/drivers/md/dm-cache-target.c
> > > +++ b/drivers/md/dm-cache-target.c
> > > @@ -3200,8 +3200,8 @@ static int parse_cblock_range(struct cache *cache, const char *str,
> > >          * Try and parse form (ii) first.
> > >          */
> > >         r = sscanf(str, "%llu-%llu%c", &b, &e, &dummy);
> > > -       if (r < 0)
> > > -               return r;
> > > +       if (r == -1)
> > > +               return -EINVAL;
> > >
> > >         if (r == 2) {
> > >                 result->begin = to_cblock(b);
> > > @@ -3213,8 +3213,8 @@ static int parse_cblock_range(struct cache *cache, const char *str,
> > >          * That didn't work, try form (i).
> > >          */
> > >         r = sscanf(str, "%llu%c", &b, &dummy);
> > > -       if (r < 0)
> > > -               return r;
> > > +       if (r == -1)
> > > +               return -EINVAL;
> > >
> > >         if (r == 1) {
> > >                 result->begin = to_cblock(b);
> >
> >
> > Could you please clarify how to reproduce unexpected results? From
> > what I observe, the kernel's sscanf doesn't return -1 on an empty
> > input. Even if a negative value other than -EINVAL is returned, it is
> > handled by the callers.
> >
> > Hank
>
> I applied the patch, but I deleted the conditions "if (r == -1) return
> -EINVAL;"
>
> sscanf in the kernel doesn't return negative numbers.
>

Do I have to send v2 with
if (r != 2)
 return -EINVAL;

or what should I do now?
> Mikulas

Best Regards,
Dipendra

Re: [PATCH] Return error code for failure input for sscanf in parse_cblock_range()

Posted by Mikulas Patocka 1 year, 4 months ago


On Mon, 23 Sep 2024, Dipendra Khadka wrote:

> Hi,
> 
> On Mon, 23 Sept 2024 at 20:55, Mikulas Patocka <mpatocka@redhat.com> wrote:
> >
> >
> >
> > On Mon, 23 Sep 2024, Ming-Hung Tsai wrote:
> >
> > > On Mon, Sep 23, 2024 at 12:47 AM Dipendra Khadka <kdipendra88@gmail.com> wrote:
> > > >
> > > > Smatch reported following:
> > > > '''
> > > > drivers/md/dm-cache-target.c:3204 parse_cblock_range() warn: sscanf doesn't return error codes
> > > > drivers/md/dm-cache-target.c:3217 parse_cblock_range() warn: sscanf doesn't return error codes
> > > > '''
> > > >
> > > > Since, the only negative value that is returned by sscanf is -1.
> > > > Returning -ENVAL when sscanf returns -1.
> > > >
> > > > Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>
> > > > ---
> > > >  drivers/md/dm-cache-target.c | 8 ++++----
> > > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
> > > > index 17f0fab1e254..c35d65e310d6 100644
> > > > --- a/drivers/md/dm-cache-target.c
> > > > +++ b/drivers/md/dm-cache-target.c
> > > > @@ -3200,8 +3200,8 @@ static int parse_cblock_range(struct cache *cache, const char *str,
> > > >          * Try and parse form (ii) first.
> > > >          */
> > > >         r = sscanf(str, "%llu-%llu%c", &b, &e, &dummy);
> > > > -       if (r < 0)
> > > > -               return r;
> > > > +       if (r == -1)
> > > > +               return -EINVAL;
> > > >
> > > >         if (r == 2) {
> > > >                 result->begin = to_cblock(b);
> > > > @@ -3213,8 +3213,8 @@ static int parse_cblock_range(struct cache *cache, const char *str,
> > > >          * That didn't work, try form (i).
> > > >          */
> > > >         r = sscanf(str, "%llu%c", &b, &dummy);
> > > > -       if (r < 0)
> > > > -               return r;
> > > > +       if (r == -1)
> > > > +               return -EINVAL;
> > > >
> > > >         if (r == 1) {
> > > >                 result->begin = to_cblock(b);
> > >
> > >
> > > Could you please clarify how to reproduce unexpected results? From
> > > what I observe, the kernel's sscanf doesn't return -1 on an empty
> > > input. Even if a negative value other than -EINVAL is returned, it is
> > > handled by the callers.
> > >
> > > Hank
> >
> > I applied the patch, but I deleted the conditions "if (r == -1) return
> > -EINVAL;"
> >
> > sscanf in the kernel doesn't return negative numbers.
> >
> 
> Do I have to send v2 with
> if (r != 2)
>  return -EINVAL;
> 
> or what should I do now?
> > Mikulas
> 
> Best Regards,
> Dipendra

You can do nothing :)

I fixed the patch and I'll send it to Linus in this merge window.

Mikulas

Re: [PATCH] Return error code for failure input for sscanf in parse_cblock_range()

Posted by Dipendra Khadka 1 year, 4 months ago

On Mon, 23 Sept 2024 at 21:45, Mikulas Patocka <mpatocka@redhat.com> wrote:
>
>
>
> On Mon, 23 Sep 2024, Dipendra Khadka wrote:
>
> > Hi,
> >
> > On Mon, 23 Sept 2024 at 20:55, Mikulas Patocka <mpatocka@redhat.com> wrote:
> > >
> > >
> > >
> > > On Mon, 23 Sep 2024, Ming-Hung Tsai wrote:
> > >
> > > > On Mon, Sep 23, 2024 at 12:47 AM Dipendra Khadka <kdipendra88@gmail.com> wrote:
> > > > >
> > > > > Smatch reported following:
> > > > > '''
> > > > > drivers/md/dm-cache-target.c:3204 parse_cblock_range() warn: sscanf doesn't return error codes
> > > > > drivers/md/dm-cache-target.c:3217 parse_cblock_range() warn: sscanf doesn't return error codes
> > > > > '''
> > > > >
> > > > > Since, the only negative value that is returned by sscanf is -1.
> > > > > Returning -ENVAL when sscanf returns -1.
> > > > >
> > > > > Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com>
> > > > > ---
> > > > >  drivers/md/dm-cache-target.c | 8 ++++----
> > > > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
> > > > > index 17f0fab1e254..c35d65e310d6 100644
> > > > > --- a/drivers/md/dm-cache-target.c
> > > > > +++ b/drivers/md/dm-cache-target.c
> > > > > @@ -3200,8 +3200,8 @@ static int parse_cblock_range(struct cache *cache, const char *str,
> > > > >          * Try and parse form (ii) first.
> > > > >          */
> > > > >         r = sscanf(str, "%llu-%llu%c", &b, &e, &dummy);
> > > > > -       if (r < 0)
> > > > > -               return r;
> > > > > +       if (r == -1)
> > > > > +               return -EINVAL;
> > > > >
> > > > >         if (r == 2) {
> > > > >                 result->begin = to_cblock(b);
> > > > > @@ -3213,8 +3213,8 @@ static int parse_cblock_range(struct cache *cache, const char *str,
> > > > >          * That didn't work, try form (i).
> > > > >          */
> > > > >         r = sscanf(str, "%llu%c", &b, &dummy);
> > > > > -       if (r < 0)
> > > > > -               return r;
> > > > > +       if (r == -1)
> > > > > +               return -EINVAL;
> > > > >
> > > > >         if (r == 1) {
> > > > >                 result->begin = to_cblock(b);
> > > >
> > > >
> > > > Could you please clarify how to reproduce unexpected results? From
> > > > what I observe, the kernel's sscanf doesn't return -1 on an empty
> > > > input. Even if a negative value other than -EINVAL is returned, it is
> > > > handled by the callers.
> > > >
> > > > Hank
> > >
> > > I applied the patch, but I deleted the conditions "if (r == -1) return
> > > -EINVAL;"
> > >
> > > sscanf in the kernel doesn't return negative numbers.
> > >
> >
> > Do I have to send v2 with
> > if (r != 2)
> >  return -EINVAL;
> >
> > or what should I do now?
> > > Mikulas
> >
> > Best Regards,
> > Dipendra
>
> You can do nothing :)
>
> I fixed the patch and I'll send it to Linus in this merge window.
>

Oh, ok. Thank you for your time.

> Mikulas

Best Regards,
Dipendra.