Re: [Rpm-maint] [rpm-software-management/rpm] Add %bcond macro for defining build conditionals (#1520)
@DemiMarie commented on this pull request. > @@ -76,47 +76,25 @@ %defined() %{expand:%%{?%{1}:1}%%{!?%{1}:0}} %undefined() %{expand:%%{?%{1}:0}%%{!?%{1}:1}} -# Shorthand for %{defined with_...} +# Handle conditional builds. +# (see 'conditionalbuilds' in the manual) +# +# Internally, the `--with foo` option defines the macro `_with_foo` and the +# `--without foo` option defines the macro `_with_foo`. You’re welcome! -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1520#discussion_r577950438___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add %bcond macro for defining build conditionals (#1520)
@encukou commented on this pull request. > @@ -76,47 +76,25 @@ %defined() %{expand:%%{?%{1}:1}%%{!?%{1}:0}} %undefined() %{expand:%%{?%{1}:0}%%{!?%{1}:1}} -# Shorthand for %{defined with_...} +# Handle conditional builds. +# (see 'conditionalbuilds' in the manual) +# +# Internally, the `--with foo` option defines the macro `_with_foo` and the +# `--without foo` option defines the macro `_with_foo`. Thank you! -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1520#discussion_r577920394___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add %bcond macro for defining build conditionals (#1520)
@encukou pushed 1 commit. eef816619d9bd225e9768aa81990fa6b36e387e8 Doc nit -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1520/files/3510e485cd647bdf211097365531af8511d90ddf..eef816619d9bd225e9768aa81990fa6b36e387e8 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Add %bcond macro for defining build conditionals (#1520)
@DemiMarie commented on this pull request. Doc nit > @@ -76,47 +76,25 @@ %defined() %{expand:%%{?%{1}:1}%%{!?%{1}:0}} %undefined() %{expand:%%{?%{1}:0}%%{!?%{1}:1}} -# Shorthand for %{defined with_...} +# Handle conditional builds. +# (see 'conditionalbuilds' in the manual) +# +# Internally, the `--with foo` option defines the macro `_with_foo` and the +# `--without foo` option defines the macro `_with_foo`. ```suggestion # `--without foo` option defines the macro `_without_foo`. ``` -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1520#pullrequestreview-592577079___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] Avoid overflow computing region offsets (#1544)
This isn’t exploitable, but it is still a good change. You can view, comment on, or merge this pull request online at: https://github.com/rpm-software-management/rpm/pull/1544 -- Commit Summary -- * Avoid overflow computing region offsets -- File Changes -- M lib/header.c (4) -- Patch Links -- https://github.com/rpm-software-management/rpm/pull/1544.patch https://github.com/rpm-software-management/rpm/pull/1544.diff -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1544 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Use int64_t for lengths (#1492)
@DemiMarie pushed 1 commit. e748c0b4297c3012c30bbcfc32ce9be904b2f2d8 Use int64_t for lengths -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1492/files/3ce3e85d61caae81d94afcff6afa5046bc2d5f65..e748c0b4297c3012c30bbcfc32ce9be904b2f2d8 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Check that ‘einfo.offset’ is reasonable (#1494)
> Nah, sprinkling checks on checks on checks when nothing bad actually happens > only makes the code less readable in the end. Again, forest from the trees. > Thanks for the detailed analysis of the case though. You’re welcome! > I'd much rather spend time treating causes than syndroms, such as looking > into eliminating those pesky signed integers from the code to the extent > possible. The region offset is the _only_ signed data in the header entries, > but signed ints are used for accessing them all over the place. What might that look like here? I’m a bit confused. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1494#issuecomment-780704610___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] ‘hdrblobInit’: check pointer is 8-byte aligned (#1499)
> On a second thought, NAK. There are too many redundant checks in the code as > it is, and adding these kind of just-in-case checks doesn't do anything to > help spot the actually crucial missing ones. Forest from the trees, you know. > > If somebody passes a random non-malloced address to headerImport() or > friends, that's a bug in the caller code. > > Apologies for asking you to do extra work and then reject anyway, but there's > simply too much on the plate as it is. That’s understandable. I will file a separate PR with a documentation fix, so that callers know what to do. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1499#issuecomment-780702430___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Avoid incrementing a pointer past the end (#1489)
> That's a generic preference BTW: comments should be used sparingly, and in > particular multiline comments are like neon signs which should be reserved > for very special danger zones. If the neon signs are everywhere, they do > nothing but blind you from the code itself. In the git era, commit messages > are a far better place to document the details. Good to know, thanks! I take it that doxygen doc comments on public APIs are an exception? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1489#issuecomment-780701176___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Avoid incrementing a pointer past the end (#1489)
@DemiMarie pushed 1 commit. c111bab0d5f8ca5e0bd2db289f0f64615945b36e Avoid incrementing a pointer past the end -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1489/files/70c3c8727d1234bf8672752dc693fbb129bca9b4..c111bab0d5f8ca5e0bd2db289f0f64615945b36e ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Clean up rdl calculation (#1488)
> Stared at this half a dozen times now, trying to break an apparent attachment > to seeing that code the way it's always been. Enough is enough :sweat_smile: > > Thanks for the patch! You’re welcome! -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1488#issuecomment-780696319___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Ensure negation comes *after* division (#1542)
> Argh, should've caught that in review > Nicely highlights how bleeping subtle this code is though - changing often > ends up just moving a trouble elsewhere. Would you be interested in OSS-Fuzz integration? Also, we might be able to use [EverParse](https://github.com/project-everest/everparse) for some of this; the parsers it generates are (IIRC) portable C and come with a proof of correctness. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1542#issuecomment-780695589___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] The RPM_INSTALL_PREFIX environment variable is missing in lua %post scriptlets (#1531)
For some more context, I'm using the relocatable packages to push Salt states into different environments, and I'm using the %post script to update symlinks within the environments. All the files in the rpms are text files. See https://docs.saltproject.io/en/latest/ref/states/top.html#multiple-environments for more info on Salt environments. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/1531#issuecomment-780578483___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Clean up rdl calculation (#1488)
Merged #1488 into master. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1488#event-4341056112___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Clean up rdl calculation (#1488)
Stared at this half a dozen times now, trying to break an apparent attachment to seeing that code the way it's always been. Enough is enough :sweat_smile: Thanks for the patch! -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1488#issuecomment-780515262___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Check that type and length are not out of range (#1491)
The type is already checked with hdrchkType() on all paths where the data comes from external sources, so that check is redundant. The length issue is deal with differently in #1492, closing this. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1491#issuecomment-780509696___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Check that len is in range before using it (#1497)
An alternative approach could be using a 64bit type for `end` so it will simply never overflow, but this is nice in that it eliminates the need for separate len test. Scrap the comment though - that deduction belongs to the commit message, not code. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1497#issuecomment-780500135___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Harden against crafted inputs (#1471)
This has been split into multiple different PR's as requested, no point keeping this open anymore. Thanks for the reproducer and all. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1471#issuecomment-780472541___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Harden against crafted inputs (#1471)
Closed #1471. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1471#event-4340721727___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Check that ‘einfo.offset’ is reasonable (#1494)
Nah, sprinkling checks on checks on checks when nothing bad actually happens only makes the code less readable in the end. Again, forest from the trees. Thanks for the detailed analysis of the case though. I'd much rather spend time treating causes than syndroms, such as looking into eliminating those pesky signed integers from the code to the extent possible. The region offset is the *only* signed data in the header entries, but signed ints are used for accessing them all over the place. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1494#issuecomment-780467828___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Check that ‘einfo.offset’ is reasonable (#1494)
Closed #1494. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1494#event-4340683537___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] ‘hdrblobInit’: check pointer is 8-byte aligned (#1499)
On a second thought, NAK. There are too many redundant checks in the code as it is, and adding these kind of just-in-case checks doesn't do anything to help spot the actually crucial missing ones. Forest from the trees, you know. If somebody passes a random non-malloced address to headerImport() or friends, that's a bug in the caller code. Apologies for asking you to do extra work and then reject anyway, but there's simply too much on the plate as it is. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1499#issuecomment-780458947___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] ‘hdrblobInit’: check pointer is 8-byte aligned (#1499)
Closed #1499. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1499#event-4340618014___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Hardlink handling cleanups + fixes (#1540)
@pmatilai pushed 1 commit. 8e5aee8368a85a8276a898aaae7026eef432e964 Add diagnostics to archive unpacking -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/rpm-software-management/rpm/pull/1540/files/59170fd2d4adff0cbf151ec616cb011ab1aca9d0..8e5aee8368a85a8276a898aaae7026eef432e964 ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Hardlink handling cleanups + fixes (#1540)
Added tests for partial hardlink sets. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1540#issuecomment-780429465___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Ensure negation comes *after* division (#1542)
Reminder to self: don't merge PR's the first thing in the morning. The comment is quite excessive for the case, an added "Negate after division to avoid overflow. Watch out for precedence!" would've been more than adequate. My bad though, and not worth changing *again* just for that. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1542#issuecomment-780407147___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Reconsider GPG key loading from %_keyringpath (#1543)
Reverting that has been discussed so many times I thought we'd actually done that by now, several years ago. Guess not... -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/1543#issuecomment-780388945___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Avoid incrementing a pointer past the end (#1489)
Actually in the latest form, the code is obvious enough that it doesn't need a comment at all. Just move the whole explanation to the commit message and we're good here. That's a generic preference BTW: comments should be used sparingly, and in particular multiline comments are like neon signs which should be reserved for very special danger zones. If the neon signs are everywhere, they do nothing but blind you from the code itself. In the git era, commit messages are a far better place to document the details. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1489#issuecomment-780386908___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
[Rpm-maint] [rpm-software-management/rpm] Reconsider GPG key loading from %_keyringpath (#1543)
At the moment rpm will load keys from a pre-defined directory (`%{_keyringpath}`) and **only** if no keys are found there, will it try to load keys from the rpmdb: https://github.com/rpm-software-management/rpm/blob/1efe530450b5bdbd90128327be56c87fa1b6843b/lib/rpmts.c#L382 This is a bit unfortunate imho, because at least as far as I am aware, no distribution really uses `%_keyringpath` to store keys there (the directory does not exist on openSUSE Tumbleweed nor on Fedora 33 and it is also not provided by any package). Now if someone drops a `*.key` file into `%_keyringpath`, they'll effectively kill key verification as everyone appears to be storing keys in the rpmdb nowadays. Therefore I would propose to revert https://github.com/rpm-software-management/rpm/commit/9d200565744d3023053d64f627c82cf2451fa701. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/issues/1543___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Ensure negation comes *after* division (#1542)
Squashed the commits as the comment is better off in the same commit to make sure they stay together in git history. This is subtle enough that it does deserve a comment. Thanks for the fix. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1542#issuecomment-780370323___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Ensure negation comes *after* division (#1542)
Merged #1542 into master. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1542#event-4339900324___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Ensure negation comes *after* division (#1542)
Argh, should've caught that in review :facepalm: Nicely highlights how bleeping subtle this code is though - changing often ends up just moving a trouble elsewhere. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1542#issuecomment-780367103___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] [rpm-software-management/rpm] Check that count won't overflow (#1493)
@pmatilai The real purpose of this check is to prevent overflows in `dataLength`. Would a check there be okay? Or would it be better to use `__builtin_mul_overflow`? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/rpm-software-management/rpm/pull/1493#issuecomment-780286447___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint