SMAP violation user-mapped address touched in get_buffers()

[korli]

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

`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 <kernel_x86_64> arch_debug_call_with_fault_handler + 0x1a
1 ffffffff81a4e2c0 (+ 80) ffffffff800b4be8 <kernel_x86_64> debug_call_with_fault_handler + 0x78
2 ffffffff81a4e310 (+ 96) ffffffff800b6294 <kernel_x86_64> kernel_debugger_loop(char const*, char const*, __va_list_tag*, int) + 0xf4
3 ffffffff81a4e370 (+ 80) ffffffff800b662e <kernel_x86_64> kernel_debugger_internal(char const*, char const*, __va_list_tag*, int) + 0x6e
4 ffffffff81a4e3c0 (+ 240) ffffffff800b69c7 <kernel_x86_64> panic + 0xb7
5 ffffffff81a4e4b0 (+ 64) ffffffff80159420 <kernel_x86_64> x86_page_fault_exception + 0x290
6 ffffffff81a4e4f0 (+ 888) ffffffff8014e7dc <kernel_x86_64> 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 <kernel_x86_64> fd_ioctl(bool, int, unsigned int, void*, unsigned long) + 0x7b
9 ffffffff81a4ef00 (+ 32) ffffffff800f014a <kernel_x86_64> _user_ioctl + 0x3a
10 ffffffff81a4ef20 (+ 16) ffffffff8014eadf <kernel_x86_64> 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 <libroot.so> _kern_ioctl + 0x0c
12 00007ffbbf218e20 (+ 16) 000000b12b82e626 <hmulti_audio.media_addon> MultiAudio::get_buffers(int, multi_buffer_list*) + 0x16
13 00007ffbbf218e30 (+ 32) 000000b12b828c47 <hmulti_audio.media_addon> MultiAudioDevice::_GetBuffers() + 0xb7
14 00007ffbbf218e50 (+ 128) 000000b12b828f85 <hmulti_audio.media_addon> MultiAudioDevice::_InitDriver() + 0x185
15 00007ffbbf218ed0 (+ 32) 000000b12b829135 <hmulti_audio.media_addon> MultiAudioDevice::MultiAudioDevice(char const*, char const*) + 0x25
16 00007ffbbf218ef0 (+ 304) 000000b12b82866b <hmulti_audio.media_addon> MultiAudioAddOn::_RecursiveScan(char const*, BEntry*, unsigned int) + 0x11b
17 00007ffbbf219020 (+ 304) 000000b12b8285f5 <hmulti_audio.media_addon> MultiAudioAddOn::_RecursiveScan(char const*, BEntry*, unsigned int) + 0xa5
18 00007ffbbf219150 (+ 48) 000000b12b8289b1 <hmulti_audio.media_addon> MultiAudioAddOn::MultiAudioAddOn(int) + 0x51
19 00007ffbbf219180 (+ 32) 000000b12b828a22 <hmulti_audio.media_addon> make_media_addon + 0x22
20 00007ffbbf2191a0 (+ 80) 000001a26d577d5e <libmedia.so> BPrivate::media::DormantNodeManager::_LoadAddOn(char const*, int, BMediaAddOn**, int*) + 0x4e
21 00007ffbbf2191f0 (+ 176) 000001a26d5780a6 <libmedia.so> 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 <libbe.so> BLooper::task_looper() + 0x294
27 00007ffbbf219730 (+ 32) 00000138cbd3ada1 <libbe.so> 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]

b3a98e8 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