bugdmidecode - Bugs: bug #55026, dmidecode --no-sysfs results...

 
 

bug #55026: dmidecode --no-sysfs results "Bus error" on ARM64 platform with performance optimized memcpy in glibc

Submitted by:  None
Submitted on:  Thu 15 Nov 2018 02:01:28 AM UTC  
 
Category:  None Severity:  3 - Normal
Item Group:  None Status:  In Progress
Privacy:  Public Assigned to:  Jean Delvare <khali>
Open/Closed:  Open

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Tue 11 Dec 2018 08:53:17 AM UTC, comment #12: 

"Other operating systems" currently means not only FreeBSD but also NetBSD, OpenBSD, BeOS, Solaris and Haiku.

Jean Delvare <khali>
Project AdministratorIn charge of this item.
Fri 16 Nov 2018 12:27:04 PM UTC, comment #11: 

That's a good point about other operating systems. I guess in practice this means FreeBSD? We (not you) should work to provide proper kernel interfaces there too.

Thank you also for pointing out SuSe's arm64 kernel config is currently suboptimal. I will talk to someone about getting that fixed.

One additional point of information that may be worth adding is that the current behaviour of SMBIOS tables being mapped with device memory attributes is not a specific decision for SMBIOS on behalf of the kernel - it is simply the current fallback for "well, this isn't part of the memblock pool". This behaviour could change to something else in the future.

I would also assume this behaviour is not the same under other operating systems, so the workarounds would not be needed for them?

Leif Lindholm <leiflindholm>
Fri 16 Nov 2018 10:17:44 AM UTC, comment #10: 

Option --no-sysfs is actually documented as being a debug option. If the sysfs interface is available, it should be used. Calling "dmidecode --no-sysfs" as part of a "test case" sounds silly to me.

For what it's worth, all arm SUSE kernels have CONFIG_DEVMEM=y. So Leif's dream to see no /dev/mem on arm is far from being the reality. Anyway, whether or not /dev/mem should exist is beyond the scope of this bug or dmidecode in general. If you care, get the discussion to distribution kernel maintainers and upstream kernel developers.

If /dev/mem exists, dmidecode (or any other tool) can try to use it, but should not cause a bus error. So for now I'm going to commit the patch I attached to this bug.

Remember that dmidecode supports operating systems other than Linux, and these operating systems don't have sysfs, so /dev/mem is the only interface which works for them.

I do have a patch ready which disables --no-sysfs (and -d) at build time. I don't want to do it by default at this point, but I'll submit it for comments and review on the dmidecode-devel list. I could commit it if some dmidecode package maintainers think it makes sense for their distribution.

Jean Delvare <khali>
Project AdministratorIn charge of this item.
Fri 16 Nov 2018 05:36:15 AM UTC, comment #9: 

1. /dev/mem is the opposite of why we have operating systems. x86 has certain other dependencies that means they cannot as easily get rid of /dev/mem as other architectures, but dmidecode no longer needs it even there. What makes it a greater issue on arm64 is that there is no standardised memory map. Which makes the linux kernel's treatment of /dev/mem less able to deal with unexpected situations. Which means that if your kernel has /dev/mem enabled in your kernel, you are actively working to make customers accidentally deadlock your systems by running normal commands as root (or as non-root if spectre/meltdown protections are not enabled or insufficient).

2. A correctly configured kernel in a multi-user arm/arm64 system will indeed not have CONFIG_DEVMEM set to y. Removing the ability to enable it at all is desirable, but somewhat hampered by existing users of embedded development boards who want to use it instead of writing device drivers.

3. Yes, --no-sysfs should ideally be disabled on anything non-x86.
There is no more value in --no-sysfs on arm64 than there would be in randomly overwriting bits of the kernel in order to see what happens. In a perfect world, --no-sysfs would only be used for dmidecode validation purposes even on x86.

Leif Lindholm <leiflindholm>
Fri 16 Nov 2018 02:56:07 AM UTC, comment #8: 

Hi Leif,

Thanks for your comment. I still have some questions:
1. If /dev/mem is dangerous on arm64, why isn't it the same on x86?
2. If /dev/mem is dangerous, why not to restrict enabling /dev/mem on arm64, for we can easily write another user program to use /dev/mem even without dmidecode?
3. If we don't want users to use --no-sysfs on arm64, we'd better disable the flag totally. Right now it returns success or "Bus error" depending on DMI table length, glibc memcpy and kernel version. And in my opinion, --no-sysfs may still be useful for debug purpose, as well as /dev/mem.

Heyi Guo <garyguo>
Fri 16 Nov 2018 02:38:45 AM UTC, comment #7: 

Hi Jean,

I tested and the patch works well.

Thanks!

Heyi Guo <garyguo>
Thu 15 Nov 2018 05:07:06 PM UTC, comment #6: 

There is no valid situation for getting this information through /dev/mem on arm/arm64 platforms (and the interface should be disabled on any multi-user system, since it opens up for many trivial denial-of-service attacks).

So my suggestion would be to not even attempt the operation on these architectures.

Leif Lindholm <leiflindholm>
Thu 15 Nov 2018 02:28:19 PM UTC, comment #5: 

Thanks for the smart suggestion. I had a solution based on aligning the initial memcpy and using an intermediate memory buffer, which was working but made the code more complex. Going for a slow manual copy may be just as good in this case, it is more simple and not necessarily slower, as it avoids the intermediate copy.

I'll attach the patch I came up with, please review and test.

Jean Delvare <khali>
Project AdministratorIn charge of this item.
Thu 15 Nov 2018 10:50:48 AM UTC, comment #4: 

That's great!

For the source may not be normal memory, how about implementing dmidecode internal version of memcpy which does not depend on glibc implementation, something like one byte copy each time? And I think the performance does not really matter here for it is only done once with no large data. It should be simpler than adjusting alignment. Just a advice.

Thanks.

Heyi Guo <garyguo>
Thu 15 Nov 2018 10:13:56 AM UTC, comment #3: 

I can't read arm64 assembly code, sorry.

When we added arm64 support to dmidecode, I was told that reading the DMI table from /dev/mem on this architecture was unsupported by design. I did not have access to any such system at the time and trusted that statement.

Now I have access to an arm64 system and it turns out that this was wrong. Reading the DMI table from /dev/mem appears to be doable, as long as we are super cautious about alignment, both for mmap() and for the following memcpy() operating on it. I'm working on a patch, I'll post it here when it is ready.

Jean Delvare <khali>
Project AdministratorIn charge of this item.
Thu 15 Nov 2018 09:14:10 AM UTC, comment #2: 

Hi Jean,

We are building a generic ARM64 OS distribution, so we have some test case to test dmidecode with --no-sysfs flag. I don't quite understand why you said it was not possible to read it from /dev/mem; from my test result we can do that by simply enabling CONFIG_DEVMEM in building kernel.

The SMBIOS table memory address is 0x3a150000 on our platform, which is page aligned, while the length is 0x361. Referring to the code https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/aarch64/memcpy.S;h=7e1163e6a0a1805877bc46defd4ee3e56a340927;hb=75c1aee500ac95bde2b800b3d787c0dd805a8a82, line 151, we can see the length does matter, for srcend comes from src + count (count is length). So srcend is not 8 bytes aligned if count is 0x361 and src is page aligned.

ARM64 platform supports unaligned access only for normal memory. For UEFI runtime services data memory, kernel will map it as "DEVICE-nGnRnE" and access must be kept aligned, or else an alignment fault exception will be triggered and "Bus error" returned to user space.

We are using dmidecode 3.0. I also tried the latest source code from git, which has the same issue. My build command is:
make -j16 'CFLAGS=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches  '

If we don't want to provide --no-sysfs on ARM64 platforms, can we totally disable this option for ARM64?

Thanks,

Gary

Heyi Guo <garyguo>
Thu 15 Nov 2018 07:49:13 AM UTC, comment #1: 

Alignment is about the base address (or offset if you prefer). It is not related to the length of the data being read. So I don't think the length of the DMI table is relevant here. What is the memory address of the DMI table?

Which version of dmidecode are you talking about? Did you build it yourself, or is it a binary package provided by someone else?

In dmidecode, mmap() is always called on a page boundary, independently of the exact address where the data starts, so I can't see how unaligned access could be the problem. On top of that, as far as I know unaligned access is actually OK on arm64 hardware.

I think the real problem here is reading the DMI table data from /dev/mem in the first place. One of the reasons why we implemented the sysfs interface to access the DMI table data was precisely because on arm64 it was not possible to read it from /dev/mem.

So I think you are just shooting yourself into the foot by passing --no-sysfs to dmidecode on arm64. Why are you doing that in the first place?

Jean Delvare <khali>
Project AdministratorIn charge of this item.
Thu 15 Nov 2018 02:01:28 AM UTC, original submission:  

Platform: ARM64 with UEFI and ACPI enabled
Linux kernel: 4.14 or 4.19
glibc: performance optimized memcpy which assumes unaligned access is allowed, see
https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/aarch64/memcpy.S;h=7e1163e6a0a1805877bc46defd4ee3e56a340927;hb=75c1aee500ac95bde2b800b3d787c0dd805a8a82

dmidecode will get table length from the firmware, which may not be well aligned; it is 0x361 on our platform. And I didn't find SMBIOS specification restricts the length alignment. Then it will mmap /dev/mem and try to read from the firmware reported address, which is UEFI runtime services data and marked as "reserved" in /proc/iomem. Kernel maps the address as DEVICE attribute, and it will generate alignment fault exception when unaligned access occurs, and the optimized memcpy actually does that for our table 0x361, for a "ldp" instruction to a non-8-bytes aligned address. In user space, it shows us with "Bus error" and terminates immediately.

So can we have some way to fix this?

Thanks,

Gary

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #45432:  slow-memcpy-on-arm64.patch added by khali (2KiB - text/x-patch - [PATCH] dmidecode: Don't use memcpy on /dev/mem on arm64)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by leiflindholm (Posted a comment)
  • -email is unavailable- added by garyguo (Posted a comment)
  • -email is unavailable- added by khali (Posted a comment)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-12-11 khali StatusNeed Info => In Progress
    2018-11-15 khali Attached File- => Added slow-memcpy-on-arm64.patch, #45432
        StatusIn Progress => Need Info
    2018-11-15 khali StatusNeed Info => In Progress
    2018-11-15 khali StatusNone => Need Info
        Assigned toNone => khali

    Back to the top


    Powered by Savane 3.5