Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

depmod: Actually use scratch buffer memory #166

Closed
wants to merge 2 commits into from

Conversation

stoeckmann
Copy link
Contributor

Accessing the backing "alias" char buffer directly works only as long as all symbols are smaller than 1024, because otherwise the scratch buffer allocates memory and the addresses do not match anymore.

Proof of Concept:

  1. Create a temporary modules directory containing a module with two symbols, aa and 1025*A (i.e. longer than 1024 characters)
MYDIR=$(mktemp -d)
KDIR=$MYDIR/lib/modules/$(uname -r)
mkdir -p $KDIR/kernel/sub
cat > $MYDIR/mod.ko.xz.b64 << EOF
/Td6WFoAAATm1rRGBMBvmQohARwAAAAAAAAAAMNcVP3gBRgAZ10AP5FFhGhEVEmcAlZtZy+BVw2o
524UVW19oueqRkgFvigdxpHbOYyTU7Kx/hlefDfD+r/tFCQlBcHuPRCbnxuRTPRldwa2DYFC2g0c
DdY1yxHvF6tv9kbWqc8xpwpQ8YWKVB8MXvUAAAAAPR9nVoEFabMAAYsBmQoAAI4ynUexxGf7AgAA
AAAEWVo=
EOF
base64 -d $MYDIR/mod.ko.xz.b64 | xz -cd > $KDIR/kernel/sub/mod.ko
  1. Run depmod with warnings enabled
depmod -b $MYDIR -o $MYDIR/out -w

You can see, among other warnings, this one:

depmod: WARNING: duplicate module syms:
symbol:aa mod

Since the 1025*A symbol name triggered an allocation within the scratch buffer, the first symbol name aa has been added twice. The 1025*A symbol name can be seen in written modules.symbols file, but not in modules.symbols.bin (because the scratch buffer issue is triggered in second output).

Copy link
Contributor

@lucasdemarchi lucasdemarchi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

On a related note since you are reviewing these things. I was thinking of blending strbuf and scratchbuf, always allowing to start with a stack buffer and if NULL is passed, then the current behavior for strbuf is retained. Any thoughts?

tools/depmod.c Show resolved Hide resolved
@stoeckmann
Copy link
Contributor Author

On a related note since you are reviewing these things. I was thinking of blending strbuf and scratchbuf, always allowing to start with a stack buffer and if NULL is passed, then the current behavior for strbuf is retained. Any thoughts?

I've had an idea like that as well, also possibly backing array creations with scratchbuf. The concept of scratchbuf is awesome, since a lot of allocations could be skipped when it comes to heuristics and common module setups in distributions.

But I didn't make any progress into that direction, so: Yes I would like to see that, but have no code to share. :D

stoeckmann and others added 2 commits September 30, 2024 20:22
Accessing the backing "alias" char buffer directly works only as long as
all symbols are smaller than 1024, because otherwise the scratch buffer
allocates memory and the addresses do not match anymore.

Signed-off-by: Tobias Stoeckmann <[email protected]>
It is meant as a backing buffer for scratch buffer. Do not use it
directly when constructing strings.

Co-authored-by: Lucas De Marchi <[email protected]>
Signed-off-by: Tobias Stoeckmann <[email protected]>
@lucasdemarchi
Copy link
Contributor

But I didn't make any progress into that direction, so: Yes I would like to see that, but have no code to share. :D

I started doing that and left saved on a branch... I will try to finish it and submit as a PR

lucasdemarchi pushed a commit that referenced this pull request Oct 1, 2024
Accessing the backing "alias" char buffer directly works only as long as
all symbols are smaller than 1024, because otherwise the scratch buffer
allocates memory and the addresses do not match anymore.

Signed-off-by: Tobias Stoeckmann <[email protected]>
Link: #166
Signed-off-by: Lucas De Marchi <[email protected]>
lucasdemarchi added a commit that referenced this pull request Oct 1, 2024
It is meant as a backing buffer for scratch buffer. Do not use it
directly when constructing strings.

Co-authored-by: Lucas De Marchi <[email protected]>
Signed-off-by: Tobias Stoeckmann <[email protected]>
Link: #166
Signed-off-by: Lucas De Marchi <[email protected]>
@lucasdemarchi
Copy link
Contributor

Applied, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants