On Fri, Apr 7, 2023 at 12:43 AM Andrii Nakryiko <andrii@xxxxxxxxxx> wrote: > > Change how we do accounting in BPF_LOG_FIXED mode and adopt log->end_pos > as *logical* log position. This means that we can go beyond physical log > buffer size now and be able to tell what log buffer size should be to > fit entire log contents without -ENOSPC. > > To do this for BPF_LOG_FIXED mode, we need to remove a short-circuiting > logic of not vsnprintf()'ing further log content once we filled up > user-provided buffer, which is done by bpf_verifier_log_needed() checks. > We modify these checks to always keep going if log->level is non-zero > (i.e., log is requested), even if log->ubuf was NULL'ed out due to > copying data to user-space, or if entire log buffer is physically full. > We adopt bpf_verifier_vlog() routine to work correctly with > log->ubuf == NULL condition, performing log formatting into temporary > kernel buffer, doing all the necessary accounting, but just avoiding > copying data out if buffer is full or NULL'ed out. > > With these changes, it's now possible to do this sort of determination of > log contents size in both BPF_LOG_FIXED and default rolling log mode. > We need to keep in mind bpf_vlog_reset(), though, which shrinks log > contents after successful verification of a particular code path. This > log reset means that log->end_pos isn't always increasing, so to return > back to users what should be the log buffer size to fit all log content > without causing -ENOSPC even in the presence of log resetting, we need > to keep maximum over "lifetime" of logging. We do this accounting in > bpf_vlog_update_len_max() helper. > > A related and subtle aspect is that with this logical log->end_pos even in > BPF_LOG_FIXED mode we could temporary "overflow" buffer, but then reset > it back with bpf_vlog_reset() to a position inside user-supplied > log_buf. In such situation we still want to properly maintain > terminating zero. We will eventually return -ENOSPC even if final log > buffer is small (we detect this through log->len_max check). This > behavior is simpler to reason about and is consistent with current > behavior of verifier log. Handling of this required a small addition to > bpf_vlog_reset() logic to avoid doing put_user() beyond physical log > buffer dimensions. > > Another issue to keep in mind is that we limit log buffer size to 32-bit > value and keep such log length as u32, but theoretically verifier could > produce huge log stretching beyond 4GB. Instead of keeping (and later > returning) 64-bit log length, we cap it at UINT_MAX. Current UAPI makes > it impossible to specify log buffer size bigger than 4GB anyways, so we > don't really loose anything here and keep everything consistently 32-bit > in UAPI. This property will be utilized in next patch. > > Doing the same determination of maximum log buffer for rolling mode is > trivial, as log->end_pos and log->start_pos are already logical > positions, so there is nothing new there. > > These changes do incidentally fix one small issue with previous logging > logic. Previously, if use provided log buffer of size N, and actual log > output was exactly N-1 bytes + terminating \0, kernel logic coun't > distinguish this condition from log truncation scenario which would end > up with truncated log contents of N-1 bytes + terminating \0 as well. > > But now with log->end_pos being logical position that could go beyond > actual log buffer size, we can distinguish these two conditions, which > we do in this patch. This plays nicely with returning log_size_actual > (implemented in UAPI in the next patch), as we can now guarantee that if > user takes such log_size_actual and provides log buffer of that exact > size, they will not get -ENOSPC in return. > > All in all, all these changes do conceptually unify fixed and rolling > log modes much better, and allow a nice feature requested by users: > knowing what should be the size of the buffer to avoid -ENOSPC. > > We'll plumb this through the UAPI and the code in the next patch. > > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx> Acked-by: Lorenz Bauer <lmb@xxxxxxxxxxxxx>
