linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [bug report] tracing: probeevent: Add array type support
@ 2023-06-26 14:33 Dan Carpenter
  2023-06-28  0:09 ` Masami Hiramatsu
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Dan Carpenter @ 2023-06-26 14:33 UTC (permalink / raw)
  To: mhiramat; +Cc: linux-trace-kernel

Hello Masami Hiramatsu,

The patch 40b53b771806: "tracing: probeevent: Add array type support"
from Apr 25, 2018, leads to the following Smatch static checker
warning:

	kernel/trace/trace_probe_tmpl.h:207 process_fetch_insn_bottom()
	warn: using error codes for math 'ret'

kernel/trace/trace_probe_tmpl.h
    122 static nokprobe_inline int
    123 process_fetch_insn_bottom(struct fetch_insn *code, unsigned long val,
    124                            void *dest, void *base)
    125 {
    126         struct fetch_insn *s3 = NULL;
    127         int total = 0, ret = 0, i = 0;
    128         u32 loc = 0;
    129         unsigned long lval = val;
    130 
    131 stage2:
    132         /* 2nd stage: dereference memory if needed */
    133         do {
    134                 if (code->op == FETCH_OP_DEREF) {
    135                         lval = val;
    136                         ret = probe_mem_read(&val, (void *)val + code->offset,
    137                                              sizeof(val));
    138                 } else if (code->op == FETCH_OP_UDEREF) {
    139                         lval = val;
    140                         ret = probe_mem_read_user(&val,
    141                                  (void *)val + code->offset, sizeof(val));
    142                 } else
    143                         break;
    144                 if (ret)
    145                         return ret;
    146                 code++;
    147         } while (1);
    148 
    149         s3 = code;
    150 stage3:
    151         /* 3rd stage: store value to buffer */
    152         if (unlikely(!dest)) {
    153                 switch (code->op) {
    154                 case FETCH_OP_ST_STRING:
    155                         ret = fetch_store_strlen(val + code->offset);
    156                         code++;
    157                         goto array;
    158                 case FETCH_OP_ST_USTRING:
    159                         ret += fetch_store_strlen_user(val + code->offset);
    160                         code++;
    161                         goto array;
    162                 case FETCH_OP_ST_SYMSTR:
    163                         ret += fetch_store_symstrlen(val + code->offset);

I think all these functions return zero on error.

    164                         code++;
    165                         goto array;
    166                 default:
    167                         return -EILSEQ;
    168                 }
    169         }
    170 
    171         switch (code->op) {
    172         case FETCH_OP_ST_RAW:
    173                 fetch_store_raw(val, code, dest);
    174                 break;
    175         case FETCH_OP_ST_MEM:
    176                 probe_mem_read(dest, (void *)val + code->offset, code->size);
    177                 break;
    178         case FETCH_OP_ST_UMEM:
    179                 probe_mem_read_user(dest, (void *)val + code->offset, code->size);
    180                 break;
    181         case FETCH_OP_ST_STRING:
    182                 loc = *(u32 *)dest;
    183                 ret = fetch_store_string(val + code->offset, dest, base);

This function return -EFAULT if copy_from_user() fails.

    184                 break;
    185         case FETCH_OP_ST_USTRING:
    186                 loc = *(u32 *)dest;
    187                 ret = fetch_store_string_user(val + code->offset, dest, base);

Same.

    188                 break;
    189         case FETCH_OP_ST_SYMSTR:
    190                 loc = *(u32 *)dest;
    191                 ret = fetch_store_symstring(val + code->offset, dest, base);

My guess is that Smatch thinks this returns -ENOMEM.

    192                 break;
    193         default:
    194                 return -EILSEQ;
    195         }
    196         code++;
    197 
    198         /* 4th stage: modify stored value if needed */
    199         if (code->op == FETCH_OP_MOD_BF) {
    200                 fetch_apply_bitfield(code, dest);
    201                 code++;
    202         }
    203 
    204 array:
    205         /* the last stage: Loop on array */
    206         if (code->op == FETCH_OP_LP_ARRAY) {
--> 207                 total += ret;

This is an unpublished check because I need to go through and make a
list of all the functions which can't fail in real life.  But the rule
here is that Smatch doesn't like adding error codes to anything.

    208                 if (++i < code->param) {
    209                         code = s3;
    210                         if (s3->op != FETCH_OP_ST_STRING &&
    211                             s3->op != FETCH_OP_ST_USTRING) {
    212                                 dest += s3->size;
    213                                 val += s3->size;
    214                                 goto stage3;
    215                         }
    216                         code--;
    217                         val = lval + sizeof(char *);
    218                         if (dest) {
    219                                 dest += sizeof(u32);
    220                                 *(u32 *)dest = update_data_loc(loc, ret);
    221                         }
    222                         goto stage2;
    223                 }
    224                 code++;
    225                 ret = total;
    226         }
    227 
    228         return code->op == FETCH_OP_END ? ret : -EILSEQ;
    229 }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-07-02 13:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-26 14:33 [bug report] tracing: probeevent: Add array type support Dan Carpenter
2023-06-28  0:09 ` Masami Hiramatsu
2023-06-29 14:13 ` [PATCH 1/2] tracing/probes: Fix to avoid double count of the string length on the array Masami Hiramatsu (Google)
2023-07-02  1:08   ` Steven Rostedt
2023-07-02 12:56     ` Masami Hiramatsu
2023-06-29 14:13 ` [PATCH 2/2] tracing/probes: Fix to exit fetching if an error is detected Masami Hiramatsu (Google)
2023-07-02  1:16   ` Steven Rostedt
2023-07-02 13:46     ` Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).