Opened 10 years ago

Closed 10 years ago

#5118 closed bug (invalid)

Haiku panics on HDA sound card

Reported by: flaggy Owned by: korli
Priority: normal Milestone: Unscheduled
Component: Drivers/Audio/HDA Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: no Platform: x86

Description

Specification

I have a lenovo r61i laptop. The sound card is an intel hda, according to linux, the codec is Conexant CX20549 (Venice). The sound card works flawlessly on linux.

Haiku revision: 34626 (compiled with gcc 4)

Error message

PANIC: vm_page_fault_unhandled page fault in kernel space at 0x0, ip 0x800dd4db

The backtrace is in the figure attached to this ticket.

Experimentation

I changed the code of the function multi_audio_control (src/add-ons/kernel/drivers/audio/hda/hda_multi_audio.cpp) to always return B_ERROR and the system was able to boot. Except for no sound, the system seemed to be working well.

I thought the error could be in one of those user_memcpy, so I kept adding return B_ERROR further down the code. Eventually I removed the return from (line 1035)

			return user_memcpy(originalChannels, channels, sizeof(multi_channel_info)
					* description.request_channel_count);

and added return B_ERROR right after it. The system was able to boot. If I would change B_ERROR to B_OK, then the same kernel panic would come up again.

I'm out of ideas of what could it be. It seems to be something wrong with multi_audio_control (or at least there's where things break badly), but at the same time, the error seems to happen only when the function returns. I may be missing something, being a C programmer with little experience in C++. Does C++ calls something on the function's data after the return?

Attachments (1)

bt.jpg (471.7 KB ) - added by flaggy 10 years ago.

Download all attachments as: .zip

Change History (10)

by flaggy, 10 years ago

Attachment: bt.jpg added

comment:1 by stippi, 10 years ago

The code looks very unhealthy. The MultiAudioDevice passes a buffer in an ioctl() which is a structure "multi_description". This structure contains a pointer to another memory location allocated in the media_addon_server memory space, "channels" of type "multi_channel_info*". Then the driver later copies to this memory location which is of course not part of the original buffer passed to the ioctl(), but points to random kernel memory in the driver address space.

comment:2 by stippi, 10 years ago

As a solution, I propose to make the channels member in multi_description part of the buffer, ie declare it as "multi_channel_info channels[16]" in the hmulti_audio.h header. But then every driver using the protocol must be checked, they probably all have the same error. Unfortunately, I don't have time to solve this bug myself, since I promised to do some christmas bakery today... :-)

comment:3 by bonefish, 10 years ago

@stippi: You must be looking at the wrong ioctl. The one that crashes is 8033 (B_MULTI_LIST_MIX_CONTROLS), i.e. list_mix_controls() or any of the inlined functions it calls. What definitely isn't good is that list_mix_controls() accesses the userland buffer it is passed unchecked (all code in that file does :-/). But that wouldn't cause the crash in strcpy(), since that doesn't use the userland buffer. It would help to build the driver with debugging enabled, so that the stack trace gets a bit more detailed.

Regarding the multi_description::channels issue you see, I don't. B_MULTI_GET_DESCRIPTION seems to be one of the few places that actually uses user_memcpy() to access user memory. Though it doesn't check whether the pointers are userland addresses in the first place, but other than that it looks OK to me.

comment:4 by stippi, 10 years ago

I may have been looking at the wrong ioctl(), but how can a driver copy something to a pointer that points to memory in the user land process? The same address will point to completely unrelated kernel memory from the driver's point of view, no?

in reply to:  4 comment:5 by anevilyak, 10 years ago

Replying to stippi:

I may have been looking at the wrong ioctl(), but how can a driver copy something to a pointer that points to memory in the user land process? The same address will point to completely unrelated kernel memory from the driver's point of view, no?

Was it not using user_memcpy() ? That function's designed to deal with precisely that case, where a pointer has to be resolved using the calling team's address space map rather than the kernel's.

in reply to:  4 comment:6 by bonefish, 10 years ago

Replying to stippi:

I may have been looking at the wrong ioctl(), but how can a driver copy something to a pointer that points to memory in the user land process? The same address will point to completely unrelated kernel memory from the driver's point of view, no?

No, the virtual address space is split. One part (on x86 currently the upper half) is used for the kernel, the other one for userland teams. The kernel part is fixed and always visible, the userland part switches with the running thread. When a userland thread enters the kernel, the userland address space for the respective team is visible and can be accessed as usual. It *should* be accessed via user_memcpy() and friends only, though, because those functions can deal with invalid addresses (e.g. null pointers) passed from userland. They will simply return an error instead of crashing the kernel.

comment:7 by korli, 10 years ago

Component: Audio & VideoDrivers/Audio/HDA
Owner: changed from nobody to korli

The number of controls can be a maximum of 128 (driver and addon). To crash and exceed this value, there is probably a loop in the widget graph.

Could you please try this ?

  1. Comment out the media server start up in the Bootscript.
  2. Enable the syslog (maybe activate in /boot/home/config/settings/kernel/drivers/kernel)
  3. Restart the system.
  4. Use "find /dev/audio/" to have the hda driver loaded.
  5. Attach the syslog to this ticket.

comment:8 by korli, 10 years ago

Any news?

comment:9 by korli, 10 years ago

Resolution: invalid
Status: newclosed

Lack of feedback, closing as invalid.

Note: See TracTickets for help on using tickets.