Diff Detail
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 17821 Build 17594: arc lint + arc unit
Event Timeline
@melifaro I'm thinking about how to make GET_PKTOPT_VAR a function or perhaps... how to make it much smaller and call a function for the bulk of the logic.
update GET_PKTOPT_VAR to properly check for pktopt change after dropping lock
Additionally, refactor a bit for readability.
@melifaro I was not able to refactor into functions but I was able to refactor into something easier to follow and more correct.
I was thinking of having something like the below one as a macro, moving the rest to the actual function.
#define GET_PKTOPT_XVAR(field, max_length) do { \
if (pktopt && pktopt->field) { \
field_offset = &(pktopt->field) - pktopt; \
error = get_pktopt_var(sopt, in6p, field_offset, &optdata, &optdatalen);\
if (error != 0) \
return (error); \
malloc_optdata = true; \
} \}
Hi Jason,
Not exactly.
My original concern was to make the code more readable and maintainable.
Unfortunately, the new version does not look more readable at all and is twice longer. If there is no way of doing this better - let's just don't touch that macro.
The only thing that really needs to be changed in this part of code is the
optdata = malloc(sopt->sopt_valsize, M_TEMP, M_WAITOK);
since it allocates kernel memory based on user-supplied size argument
Fix GET_PKTOPT_VAR bugs with memory allocation.
- properly detect and limit memory allocation based on user supplied argument
- properly detect and recover from requested information length changing after the in6p lock is reacquired
I agree that using less macros didn't particularly help with readability. But there are in fact two problems with the current code. The issue you pointed out with regard to protecting a user space controlled kernel memory allocation, and secondly, properly detecting and recovering from the requested data length changing when the in6p is reacquired.
I believe the now current state of the patch is the correct next step. I also believe that GET_PKTOPT_VAR is the least difficult to follow of all of the evaluated variants with this method.