SMAP violation user-mapped address touched in get_buffers()

[korli]

This time in get_buffers() instead of get_desc(). This is after 6501f1682298f336764645fe70554381d435de64

`PANIC: SMAP violation user-mapped address 0x000010edcc7bc298 touched from kernel 0xffffffff813bedbe

Welcome to Kernel Debugging Land... Thread 399 "media_addon_server" running on CPU 1 stack trace for thread 399 "media_addon_server" kernel stack: 0xffffffff81a4a000 to 0xffffffff81a4f000 user stack: 0x00007ffbbe21a000 to 0x00007ffbbf21a000 frame caller :function + offset 0 ffffffff81a4e2a0 (+ 32) ffffffff8014cf00 arch_debug_call_with_fault_handler + 0x1a 1 ffffffff81a4e2c0 (+ 80) ffffffff800b4be8 debug_call_with_fault_handler + 0x78 2 ffffffff81a4e310 (+ 96) ffffffff800b6294 kernel_debugger_loop(char const, char const, va_list_tag, int) + 0xf4 3 ffffffff81a4e370 (+ 80) ffffffff800b662e kernel_debugger_internal(char const, char const*, va_list_tag, int) + 0x6e 4 ffffffff81a4e3c0 (+ 240) ffffffff800b69c7 panic + 0xb7 5 ffffffff81a4e4b0 (+ 64) ffffffff80159420 x86_page_fault_exception + 0x290 6 ffffffff81a4e4f0 (+ 888) ffffffff8014e7dc int_bottom + 0x80 kernel iframe at 0xffffffff81a4e868 (end = 0xffffffff81a4e930) rax 0x0 rbx 0xffffffff974db7e0 rcx 0x1 rdx 0xffffffff813c3004 rsi 0x4 rdi 0xffffffff81a4ea30 rbp 0xffffffff81a4eea0 r8 0x10edcc7bc298 r9 0x0 r10 0x4 r11 0xffffffff813c3004 r12 0x10edcc7b8820 r13 0x0 r14 0xffffffff81a4ec70 r15 0xffffffff9774c7a8 rip 0xffffffff813bedbe rsp 0xffffffff81a4e930 rflags 0x10282 vector: 0xe, error code: 0x3 7 ffffffff81a4e868 (+1592) ffffffff813bedbe </boot/system/add-ons/kernel/drivers/audio/hmulti/virtio_sound> multi_audio_control_generic[clone .isra.0] (VirtIOSoundDriverInfo, unsigned int, void, unsigned long) + 0x31e 8 ffffffff81a4eea0 (+ 96) ffffffff800ef0eb fd_ioctl(bool, int, unsigned int, void, unsigned long) + 0x7b 9 ffffffff81a4ef00 (+ 32) ffffffff800f014a _user_ioctl + 0x3a 10 ffffffff81a4ef20 (+ 16) ffffffff8014eadf x86_64_syscall_entry + 0xfb user iframe at 0xffffffff81a4ef30 (end = 0xffffffff81a4eff8) rax 0x99 rbx 0x0 rcx 0x1630fcdfc2c rdx 0x10edcc7b8820 rsi 0x1f63 rdi 0xc rbp 0x7ffbbf218e20 r8 0x10edcc7c8298 r9 0x2d r10 0x60 r11 0x202 r12 0x7ffbbf218e70 r13 0x10edcc7b7ea0 r14 0x2 r15 0x2 rip 0x1630fcdfc2c rsp 0x7ffbbf218e08 rflags 0x202 vector: 0x63, error code: 0x0 11 ffffffff81a4ef30 (+140721340063472) 000001630fcdfc2c _kern_ioctl + 0x0c 12 00007ffbbf218e20 (+ 16) 000000b12b82e626 MultiAudio::get_buffers(int, multi_buffer_list) + 0x16 13 00007ffbbf218e30 (+ 32) 000000b12b828c47 MultiAudioDevice::_GetBuffers() + 0xb7 14 00007ffbbf218e50 (+ 128) 000000b12b828f85 MultiAudioDevice::_InitDriver() + 0x185 15 00007ffbbf218ed0 (+ 32) 000000b12b829135 MultiAudioDevice::MultiAudioDevice(char const, char const) + 0x25 16 00007ffbbf218ef0 (+ 304) 000000b12b82866b MultiAudioAddOn::_RecursiveScan(char const, BEntry, unsigned int) + 0x11b 17 00007ffbbf219020 (+ 304) 000000b12b8285f5 MultiAudioAddOn::_RecursiveScan(char const, BEntry, unsigned int) + 0xa5 18 00007ffbbf219150 (+ 48) 000000b12b8289b1 MultiAudioAddOn::MultiAudioAddOn(int) + 0x51 19 00007ffbbf219180 (+ 32) 000000b12b828a22 make_media_addon + 0x22 20 00007ffbbf2191a0 (+ 80) 000001a26d577d5e BPrivate::media::DormantNodeManager::_LoadAddOn(char const, int, BMediaAddOn*, int) + 0x4e 21 00007ffbbf2191f0 (+ 176) 000001a26d5780a6 BPrivate::media::DormantNodeManager::GetAddOn(int) + 0x86 22 00007ffbbf2192a0 (+ 208) 000001ac55f774d2 <_APP_> MediaAddonServer::_AddOnAdded(char const, long) + 0x42 23 00007ffbbf219370 (+ 208) 000001ac55f77830 <_APP_> MediaAddonServer::MonitorHandler::AddOnEnabled(BPrivate::Storage::add_on_entry_info const) + 0xa0 24 00007ffbbf219440 (+ 640) 000001ac55f7a484 <_APP_> BPrivate::Storage::AddOnMonitorHandler::_HandlePendingEntries() + 0x94 25 00007ffbbf2196c0 (+ 32) 000001ac55f7a675 <_APP_> BPrivate::Storage::AddOnMonitorHandler::MessageReceived(BMessage*) + 0x25 26 00007ffbbf2196e0 (+ 80) 00000138cbd45e44 BLooper::task_looper() + 0x294 27 00007ffbbf219730 (+ 32) 00000138cbd3ada1 BApplication::Run() + 0x21 28 00007ffbbf219750 (+ 32) 000001ac55f75a39 <_APP_> main + 0x39 29 00007ffbbf219770 (+ 48) 000001ac55f75bce <_APP_> _start + 0x3e 30 00007ffbbf2197a0 (+ 48) 00000170b7559cd5 /boot/system/runtime_loader@0x00000170b754a000 + 0xfcd5 31 00007ffbbf2197d0 (+ 0) 00007ff58e299258 commpage_thread_exit + 0x00`

[diegoroux]

Fixing this in next commit

[diegoroux]

fixed as (in next commit):

struct buffer_desc buf_desc[stream->channels];

for (uint32 ch_id = 0; ch_id < stream->channels; ch_id++) {
  buf_desc[ch_id].base = buf_ptr + (format_size * ch_id);
  buf_desc[ch_id].stride = format_size * stream->channels;
}

status = user_memcpy(buffers[buf_id], buf_desc, sizeof(struct buffer_desc) * stream->channels);

get_buffers() was directly writing into a user-space mapped address. It wrongly assumed that the buffers_desc offered were not in user-space.

[diegoroux]

Also, now I'm using: qemu-system-x86_64 [...] -cpu Broadwell,+smap to ensure this doesn't occur again.

[diegoroux]

https://github.com/diegoroux/haiku/commit/b3a98e80c4b5b05d331d0b4b6ad1dcc14d1e58d6 should fix this issue, please let me know

[korli]

actually this isn't correct. the txArea should only be for user buffers (tx_size = stream->period_size BUFFERS). struct virtio_snd_pcm_xfer and struct virtio_snd_pcm_status should be placed in a kernel-only area, one for each buffer (xfer_size = (sizeof(struct virtio_snd_pcm_xfer) + sizeof(struct virtio_snd_pcm_status)) BUFFERS). It means using user_memcpy() isn't needed for such structs. "physical_entry entries[3]" can be directly allocated on the stack in send_playback_buffer():

       entries[0].address = info->xferAddr + (sizeof(struct virtio_snd_pcm_xfer) + sizeof(struct virtio_snd_pcm_status)) * stream-buffer_cycle;
       entries[0].size = sizeof(struct virtio_snd_pcm_xfer);
       entries[1].address = info->txAddr + stream->period_size * stream->buffer_cycle;
       entries[1].size = stream->period_size;
       entries[2].address = entries[0].address + sizeof(struct virtio_snd_pcm_xfer);
       entries[2].size = sizeof(struct virtio_snd_pcm_status);

[diegoroux]

Will change to this