Re: [Rpm-maint] [rpm-software-management/rpm] Add %bcond macro for defining build conditionals (#1520)

2021-02-17 Thread Demi Marie Obenour
@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)

2021-02-17 Thread Petr Viktorin
@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)

2021-02-17 Thread Petr Viktorin
@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)

2021-02-17 Thread Demi Marie Obenour
@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)

2021-02-17 Thread Demi Marie Obenour
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)

2021-02-17 Thread Demi Marie Obenour
@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)

2021-02-17 Thread Demi Marie Obenour
> 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)

2021-02-17 Thread Demi Marie Obenour
> 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)

2021-02-17 Thread Demi Marie Obenour
> 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)

2021-02-17 Thread Demi Marie Obenour
@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)

2021-02-17 Thread Demi Marie Obenour
> 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)

2021-02-17 Thread Demi Marie Obenour
> 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)

2021-02-17 Thread Jude N
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)

2021-02-17 Thread Panu Matilainen
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)

2021-02-17 Thread Panu Matilainen
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)

2021-02-17 Thread Panu Matilainen
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)

2021-02-17 Thread Panu Matilainen
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)

2021-02-17 Thread Panu Matilainen
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)

2021-02-17 Thread Panu Matilainen
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)

2021-02-17 Thread Panu Matilainen
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)

2021-02-17 Thread Panu Matilainen
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)

2021-02-17 Thread Panu Matilainen
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)

2021-02-17 Thread Panu Matilainen
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)

2021-02-17 Thread Panu Matilainen
@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)

2021-02-17 Thread Panu Matilainen
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)

2021-02-17 Thread Panu Matilainen
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)

2021-02-17 Thread Panu Matilainen
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)

2021-02-17 Thread Panu Matilainen
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)

2021-02-17 Thread Dan Čermák
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)

2021-02-17 Thread Panu Matilainen
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)

2021-02-17 Thread Panu Matilainen
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)

2021-02-17 Thread Panu Matilainen
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)

2021-02-17 Thread Demi Marie Obenour
@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