public inbox for smatch@vger.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Cc: linux-s390@vger.kernel.org, smatch@vger.kernel.org
Subject: Re: smatch and copy_{to,from}_user return values
Date: Fri, 5 Mar 2021 13:14:34 +0300	[thread overview]
Message-ID: <20210305101434.GI2222@kadam> (raw)
In-Reply-To: <20210303112046.GB2222@kadam>

It turns out that my check for returning -EIO instead of -EFAULT doesn't
work at all...  :/  How the cross function DB works is that it tries to
figure out groups of states which should go together.  Most of the time
you could combine all the failure paths together and combine the success
paths together, for example.

But with copy_from_user() there is only one set of states recorded.

sound/pci/rme9652/hdspm.c | copy_from_user | 1093 | 0-u32max[<=$2]|        INTERNAL | -1 |                      | ulong(*)(void*, void*, ulong) | 
sound/pci/rme9652/hdspm.c | copy_from_user | 1093 | 0-u32max[<=$2]|     CAPPED_DATA | -1 |                    $ |                      | 
sound/pci/rme9652/hdspm.c | copy_from_user | 1093 | 0-u32max[<=$2]| UNTRACKED_PARAM |  0 |                    $ |                      | 
sound/pci/rme9652/hdspm.c | copy_from_user | 1093 | 0-u32max[<=$2]| UNTRACKED_PARAM |  1 |                    $ |                      | 
sound/pci/rme9652/hdspm.c | copy_from_user | 1093 | 0-u32max[<=$2]|     PARAM_LIMIT |  2 |                    $ |             1-u64max | 
sound/pci/rme9652/hdspm.c | copy_from_user | 1093 | 0-u32max[<=$2]| NO_OVERFLOW_SIMPLE |  0 |                    $ |                      | 
sound/pci/rme9652/hdspm.c | copy_from_user | 1093 | 0-u32max[<=$2]|        STMT_CNT | -1 |                      |                   16 |

I could modify smatch_data/db/fixup_kernel.sh to hard code the desired
split, which is sort of awkward and also this in inlined so that makes
even more awkward.  Or I could create a new table with a manual way of
forcing splits in return states with entries like:

copy_from_user 0-u32max[<=$2] 0 1-u32max[<=$2]

That's probably the way to go, actually.

The check for propagating the return from copy_from_user() only looks
at assignments.  It sets the state to &remaining intialialy and then
if it sees a comparison with "if (ret) " it set the false path to &ok.
Then if we "return ret;" and "ret" is in state &remaining then complain.


static void match_copy(const char *fn, struct expression *expr, void *unused)
{
        if (expr->op == SPECIAL_SUB_ASSIGN)
                return;
        set_state_expr(my_id, expr->left, &remaining);
}

static void match_condition(struct expression *expr)
{
        if (!get_state_expr(my_id, expr))
                return;
        /* If the variable is zero that's ok */
        set_true_false_states_expr(my_id, expr, NULL, &ok);
}

regards,
dan carpenter

  reply	other threads:[~2021-03-05 10:14 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-03  7:50 smatch and copy_{to,from}_user return values Rasmus Villemoes
2021-03-03 11:20 ` Dan Carpenter
2021-03-05 10:14   ` Dan Carpenter [this message]
2021-03-10 10:01     ` Dan Carpenter
2021-03-04 18:35 ` Heiko Carstens

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210305101434.GI2222@kadam \
    --to=dan.carpenter@oracle.com \
    --cc=linux-s390@vger.kernel.org \
    --cc=rasmus.villemoes@prevas.dk \
    --cc=smatch@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox