Attention is currently required from: Nico Huber, Miklós Márton, Anastasia Klimchuk.

View Change

4 comments:

• File ni845x_spi.c:

  • Patch Set #1, Line 188: strtol

    Use `strtoul()` instead?

  • Patch Set #1, Line 483: const

    Unrelated change?

  • Patch Set #1, Line 558: strlen(CS_str)

    For another patch: This is an use-after-free bug.

  • Patch Set #1, Line 556:

    CS_number = CS_str[0] - '0';
    		free(CS_str);
    		if (strlen(CS_str) > 1 || 7 < CS_number) {
    			msg_perr("Only CS 0-7 supported\n");
    			return 1;
    		}
    

    The assignment to `CS_number` can underflow. I'd do the parsing as follows:

     	if (CS_str) {
     		if (CS_str[0] >= '0' && CS_str[0] <= '7' && CS_str[1] == '\0') {
     			CS_number = CS_str[0] - '0';
     			free(CS_str);
     		} else {
     			free(CS_str);
     			msg_perr("Only CS 0-7 supported\n");
     			return 1;
     		}
     	}

    The evaluation order of the three checks in the inner if-block is very important. The first check ensures that the length of `CS_str` is not zero, so that the third check doesn't cause undefined behavior.

To view, visit change 56637. To unsubscribe, or for help writing mail filters, visit settings.