On 11/10/13 16:38, Joe Perches wrote: > On Fri, 2013-10-11 at 16:31 +1100, Ryan Mallon wrote: >> On 11/10/13 16:25, Joe Perches wrote: >>> Printing kernel pointers via %pK has a minor defect when >>> kptr_restrict is set to 2: the pointer may be emitted >>> as "pK-error" instead of all 0's when in an interrupt. >> >> NAK. This is not a defect, as I explained earlier. It is really a defect >> that it _doesn't_ print 'pK-error' in all cases. 'pK-error' is for >> finding kernel bugs. > > Not my understanding. > > There is no bug to find when emitting a pointer via %pK. Yes there is. %pK is not valid from interrupt context, because the current way it is checked is by checking CAP_SYSLOG. It is irrelevant that this check is only done when kptr_restrict=1, using %pK from interrupt handlers is still wrong. We want to notify developers of any wrong cases. > > The only issue is that has_capability_noaudit can not > be called from an interrupt. Right. So any code that uses %pK from interrupt context would be, by definition, broken. That is what the check is looking for. It doesn't matter what the value of kptr_restrict happens to be, the code is still broken. So, with your patch, values 0 and 2 of kptr_restrict will print a seemingly correct value, but when kptr_restrict is 1 then it will print 'pK-error'. That is confusing to users: file prints correct pointer values when kptr_restrict=0; file prints 'pK-error' when kptr_restrict=1, even though I am root. Also, because the default is now kptr_restrict=0, less people will see the 'pK-error' value, so buggy code may go over-looked. It should print 'pK-error' in all cases, so that any bugs where %pK is being used from interrupt content are identified regardless of the setting of kptr_restrict. Anyway, with the approach that Eric and George suggested, this would become a non-issue. So probably just best to leave the code as is. ~Ryan -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
