Powered by Linux
Re: smatch and copy_{to,from}_user return values — Semantic Matching Tool

Re: smatch and copy_{to,from}_user return values

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux