Hello,
I want to ask something about atags_proc.c implementation. Currently,
it uses a buffer to temporarily store atags. The buffer size is set to
1.5kb for some reason, but as far as I know, atag list's size is not
limited in any way. I've got a device (HTC One) which uses about 12kb
of tags, that means it panics during boot if CONFIG_ATAGS_PROC is
enabled, because the buffer contains only part of the tag list without
an end tag.
Why is only 1.5kb used? Is it some specific size the device should not
exceed?
I don't know much about the way ARM boot process works, but I tried to
store just the pointer to atag list, and it works fine (quick patch
attached). Do atags get erased later in boot process on some platforms,
is that the reason why buffer had to be used?
This requires modifications in kexec-tools, because it uses that 1.5kb
buffer, too. Again, here's my version of such modification[1]. If this
is okay, I can create a proper patch and submit it.
Yours,
Vojtech Bocek
[1]: https://github.com/Tasssadar/kexec-tools/commit/c6844e1ddb13a6b60cfefcb01c3843da97d6174c
---
diff --git a/arch/arm/kernel/atags_proc.c b/arch/arm/kernel/atags_proc.c
index c7ff807..12cc483 100644
--- a/arch/arm/kernel/atags_proc.c
+++ b/arch/arm/kernel/atags_proc.c
@@ -21,12 +21,11 @@ static const struct file_operations atags_fops = {
.llseek = default_llseek,
};
-#define BOOT_PARAMS_SIZE 1536
-static char __initdata atags_copy[BOOT_PARAMS_SIZE];
+static const struct tag* __initdata atags_copy;
void __init save_atags(const struct tag *tags)
{
- memcpy(atags_copy, tags, sizeof(atags_copy));
+ atags_copy = tags;
}
static int __init init_atags_procfs(void)
@@ -40,7 +39,7 @@ static int __init init_atags_procfs(void)
struct buffer *b;
size_t size;
- if (tag->hdr.tag != ATAG_CORE) {
+ if (!atags_copy || tag->hdr.tag != ATAG_CORE) {
printk(KERN_INFO "No ATAGs?");
return -EINVAL;
}
@@ -49,7 +48,7 @@ static int __init init_atags_procfs(void)
;
/* include the terminating ATAG_NONE */
- size = (char *)tag - atags_copy + sizeof(struct tag_header);
+ size = (char *)tag - (char *)atags_copy + sizeof(struct tag_header);
WARN_ON(tag->hdr.tag != ATAG_NONE);
On Fri, Sep 27, 2013 at 10:25:45PM +0200, Vojtech Bocek wrote:
> I want to ask something about atags_proc.c implementation. Currently,
> it uses a buffer to temporarily store atags. The buffer size is set to
> 1.5kb for some reason, but as far as I know, atag list's size is not
> limited in any way. I've got a device (HTC One) which uses about 12kb
> of tags, that means it panics during boot if CONFIG_ATAGS_PROC is
> enabled, because the buffer contains only part of the tag list without
> an end tag.
The tags are supposed to be a short-lived structure which gets used to
pass barest minimum of details to the kernel, and the data stored in
them almost certainly gets overwritten before the kernels memory
allocators are up and running.
So, we need to statically allocate some space to save these things -
it can't be done dynamically.
The problem is this: for the vast majority of platforms, they pass no
more than 1.5kB (lower case b is *bits* not *bytes*) to the kernel in
their tagged list. Having a static allocation of 12k would be wasteful
for the majority of users.
> I don't know much about the way ARM boot process works, but I tried to
> store just the pointer to atag list, and it works fine (quick patch
> attached). Do atags get erased later in boot process on some platforms,
> is that the reason why buffer had to be used?
This may appear to work, but check it after you've been running for
some time and have exercised the memory systems. You'll probably find
that your tags have vanished!
It only needs to survive until init_atags_procfs is called, because it is
copied to another buffer for procfs entry. Can I be sure it survives until
that? My guess is that it is likely to survive, but not certain.
I suppose it is possible to somehow protect that bit of ram until it is
read by init_atags_procfs, but I wonder if you even want to do that in
mainline - if majority of devices doesn't use such big tag lists, then
it is probably that device's vendor problem. I've met this problem on two
devices so far though, both of them Android phones, one is the HTC One
(that is MSM APQ8064 SoC) and I unfortunately can't remember the first
one - I discarded it as usual Android kernel's mess.
On 27.9.2013 22:47, Russell King - ARM Linux wrote:
> On Fri, Sep 27, 2013 at 10:25:45PM +0200, Vojtech Bocek wrote:
>> I want to ask something about atags_proc.c implementation. Currently,
>> it uses a buffer to temporarily store atags. The buffer size is set to
>> 1.5kb for some reason, but as far as I know, atag list's size is not
>> limited in any way. I've got a device (HTC One) which uses about 12kb
>> of tags, that means it panics during boot if CONFIG_ATAGS_PROC is
>> enabled, because the buffer contains only part of the tag list without
>> an end tag.
>
> The tags are supposed to be a short-lived structure which gets used to
> pass barest minimum of details to the kernel, and the data stored in
> them almost certainly gets overwritten before the kernels memory
> allocators are up and running.
>
> So, we need to statically allocate some space to save these things -
> it can't be done dynamically.
>
> The problem is this: for the vast majority of platforms, they pass no
> more than 1.5kB (lower case b is *bits* not *bytes*) to the kernel in
> their tagged list. Having a static allocation of 12k would be wasteful
> for the majority of users.
>
>> I don't know much about the way ARM boot process works, but I tried to
>> store just the pointer to atag list, and it works fine (quick patch
>> attached). Do atags get erased later in boot process on some platforms,
>> is that the reason why buffer had to be used?
>
> This may appear to work, but check it after you've been running for
> some time and have exercised the memory systems. You'll probably find
> that your tags have vanished!
On Fri, Sep 27, 2013 at 11:09:13PM +0200, Vojtech Bocek wrote:
> It only needs to survive until init_atags_procfs is called, because it is
> copied to another buffer for procfs entry. Can I be sure it survives until
> that? My guess is that it is likely to survive, but not certain.
>
> I suppose it is possible to somehow protect that bit of ram until it is
> read by init_atags_procfs, but I wonder if you even want to do that in
> mainline - if majority of devices doesn't use such big tag lists, then
> it is probably that device's vendor problem. I've met this problem on two
> devices so far though, both of them Android phones, one is the HTC One
> (that is MSM APQ8064 SoC) and I unfortunately can't remember the first
> one - I discarded it as usual Android kernel's mess.
We've been through several early allocators - particularly one which
allocates from the bottom of memory upwards, which would overwrite the
ATAGs long before init_atags_procfs gets called.
If we rely on the behaviour of the current early allocator not to
touch that, and it changes in the future, that's going to be rather
too fragile.
Okay then, I suppose there is no nice way around that, or at least none
that I can find. I'll just make that initial buffer 12kb big on my kernel
for that device and be done with it.
Anyway, thanks for the information and help, it is much appreciated.
On 27.9.2013 23:15, Russell King - ARM Linux wrote:
> On Fri, Sep 27, 2013 at 11:09:13PM +0200, Vojtech Bocek wrote:
>> It only needs to survive until init_atags_procfs is called, because it is
>> copied to another buffer for procfs entry. Can I be sure it survives until
>> that? My guess is that it is likely to survive, but not certain.
>>
>> I suppose it is possible to somehow protect that bit of ram until it is
>> read by init_atags_procfs, but I wonder if you even want to do that in
>> mainline - if majority of devices doesn't use such big tag lists, then
>> it is probably that device's vendor problem. I've met this problem on two
>> devices so far though, both of them Android phones, one is the HTC One
>> (that is MSM APQ8064 SoC) and I unfortunately can't remember the first
>> one - I discarded it as usual Android kernel's mess.
>
> We've been through several early allocators - particularly one which
> allocates from the bottom of memory upwards, which would overwrite the
> ATAGs long before init_atags_procfs gets called.
>
> If we rely on the behaviour of the current early allocator not to
> touch that, and it changes in the future, that's going to be rather
> too fragile.
On Fri, Sep 27, 2013 at 10:47 PM, Russell King - ARM Linux
<[email protected]> wrote:
> On Fri, Sep 27, 2013 at 10:25:45PM +0200, Vojtech Bocek wrote:
>> I want to ask something about atags_proc.c implementation. Currently,
>> it uses a buffer to temporarily store atags. The buffer size is set to
>> 1.5kb for some reason, but as far as I know, atag list's size is not
>> limited in any way. I've got a device (HTC One) which uses about 12kb
>> of tags, that means it panics during boot if CONFIG_ATAGS_PROC is
>> enabled, because the buffer contains only part of the tag list without
>> an end tag.
>
> The tags are supposed to be a short-lived structure which gets used to
> pass barest minimum of details to the kernel, and the data stored in
> them almost certainly gets overwritten before the kernels memory
> allocators are up and running.
>
> So, we need to statically allocate some space to save these things -
> it can't be done dynamically.
>
> The problem is this: for the vast majority of platforms, they pass no
> more than 1.5kB (lower case b is *bits* not *bytes*) to the kernel in
> their tagged list. Having a static allocation of 12k would be wasteful
> for the majority of users.
It's __initdata memory, right?
So it would waste this 12 KiB only temporarily.
Stll, it enlarges the kernel image by 12 KiB, as there's no such thing
as __initbss.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds