[Bug 1938977] New: Move sssd from PCRE to PCRE2

https://bugzilla.redhat.com/show_bug.cgi?id=1938977

Bug ID: 1938977
           Summary: Move sssd from PCRE to PCRE2
           Product: Fedora
           Version: rawhide
            Status: NEW
         Component: sssd
          Keywords: FutureFeature
          Assignee: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail], [e-mail],
                    [e-mail], [e-mail],
                    [e-mail], [e-mail],
                    [e-mail], [e-mail],
                    [e-mail]
  Target Milestone: ---
    Classification: Fedora

This is a kindly hint that sssd-0:2.4.2-3.fc35 still uses PCRE library which
has been superseded with PCRE2 project by PCRE upstream in 2015. PCRE upstream
considers PCRE obsolete now and does not devote any resources to PCRE except of
critical bugs. Please consider porting sssd from PCRE to PCRE2.

[Bug 1938977] Move sssd from PCRE to PCRE2

https://bugzilla.redhat.com/show_bug.cgi?id=1938977

Tomas Halman  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #1 from Tomas Halman  ---
SSSD already supports PCRE2 library. It is just matter of packaging.

But PCRE is still the default in ./configure. Maybe it is the time to reverse
the condition and prefer PCRE2 over PCRE.

Tomas

[Bug 1938977] Move sssd from PCRE to PCRE2

https://bugzilla.redhat.com/show_bug.cgi?id=1938977

--- Comment #2 from Sumit Bose  ---
Hi,

I think an easy fix for the time being would be to replace 'BuildRequires:
pcre-devel' with 'BuildRequires: pcre2-devel' (and hope that pcre-devel is not
installed in the build environment by other means). However, given that (from
the Red Hat perspective) only RHEL-6 does not provide pcre2 and there are no
plan to make a recent version of SSSD available on this platform, I think we
can deprecate pcre1 support from SSSD upstream completely in the next major
release and remove it in the following one.

bye,
Sumit

[Bug 1938977] Move sssd from PCRE to PCRE2

https://bugzilla.redhat.com/show_bug.cgi?id=1938977

Alexey Tikhonov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |Triaged
           Assignee|[e-mail] |[e-mail]
                   |raproject.org               |

[Bug 1938977] Move sssd from PCRE to PCRE2

https://bugzilla.redhat.com/show_bug.cgi?id=1938977

Alexey Tikhonov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

[Bug 1938977] Move sssd from PCRE to PCRE2

https://bugzilla.redhat.com/show_bug.cgi?id=1938977

Alexey Tikhonov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |needinfo?([e-mail]
                   |                            |)

--- Comment #3 from Alexey Tikhonov  ---
Hi Petr,

while running our test-suite with pcre2, I run into following issue reported by
Valgrind:
```
==18073== 316 bytes in 1 blocks are possibly lost in loss record 158 of 195
==18073==    at 0x483880B: malloc (vg_replace_malloc.c:309)
==18073==    by 0x5932151: pcre2_compile_8 (in /usr/lib64/libpcre2-8.so.0.9.0)
```

do you have any idea what could be a reason?
This happens in several tests and it's always 316 bytes.

`pcre2_code_free()` should be executed, is this enough?
Or is this a "known issues"?

[Bug 1938977] Move sssd from PCRE to PCRE2

https://bugzilla.redhat.com/show_bug.cgi?id=1938977

Petr Pisar  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|needinfo?([e-mail] |
                   |)                           |

--- Comment #4 from Petr Pisar  ---
(In reply to Alexey Tikhonov from comment #3)
> while running our test-suite with pcre2, I run into following issue reported
> by Valgrind:
> ==18073== 316 bytes in 1 blocks are possibly lost in loss record 158 of 195
> ==18073==    at 0x483880B: malloc (vg_replace_malloc.c:309)
> ==18073==    by 0x5932151: pcre2_compile_8 (in
> /usr/lib64/libpcre2-8.so.0.9.0)
>
> do you have any idea what could be a reason?
> This happens in several tests and it's always 316 bytes.
>
> `pcre2_code_free()` should be executed, is this enough?
> Or is this a "known issues"?

I'm not aware about any memory leak in pcre2. Can you show me your code
including
a pattern and the options passed to pcre2_compile()? In general, once you don't
need the pcre2_code structure, you should free it with pcre2_code_free().

[Bug 1938977] Move sssd from PCRE to PCRE2

https://bugzilla.redhat.com/show_bug.cgi?id=1938977

--- Comment #5 from Alexey Tikhonov  ---
Well, 316 bytes corresponds to the pattern (and perhaps options) used:
```
"(((?P<domain>[^\\]+)\\(?P<name>.+$))|" \
"((?P<name>[^@]+)@(?P<domain>.+$))|" \
"(^(?P<name>[^@\\]+)$))";
```

But I was unable to reproduce an issue with a small test.
Seems something wrong with usage in sssd code.
I added a patch with a fix of potential mem leak to
https://github.com/SSSD/sssd/pull/5593, but so far not sure if it was a
cause...

[Bug 1938977] Move sssd from PCRE to PCRE2

https://bugzilla.redhat.com/show_bug.cgi?id=1938977

--- Comment #6 from Alexey Tikhonov  ---
Sorted out. PR is ready for review upstream.

[Bug 1938977] Move sssd from PCRE to PCRE2

https://bugzilla.redhat.com/show_bug.cgi?id=1938977

Alexey Tikhonov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |POST
         Whiteboard|sync-to-jira review         |sync-to-jira

--- Comment #7 from Alexey Tikhonov  ---
Pushed PR: https://github.com/SSSD/sssd/pull/5593

* `master`
    * f2bcf74c43a682897e586eeb775c4bfedd95bafa - sssd.supp: suppress false
positive valgrind warning about 'pcre2_code' ptr
    * 0fbe5af1f1e0b82cd36a8178e58d79b7dc357ab6 - util/regexp: regular talloc
d-tor shouldn't fail
    * 9aa6fb34bea891e0385a9fc77f0181d01984c212 - tests/test_nested_groups: mem
leak fixed
    * 31bcb6f032c326d62fb7ac5efcf2ff55c9acbe04 - tests/test_dp_opts: mem leak
fixed
    * 519d943424ee744ecf7418df6bf6f0688a7d9099 - util/regexp: local functions
shall be static
    * cbfccb173d1cfa631350778abbee82bca1fbc296 - BUILD: prefer PCRE2 over PCRE

[Bug 1938977] Move sssd from PCRE to PCRE2

https://bugzilla.redhat.com/show_bug.cgi?id=1938977

Alexey Tikhonov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|POST                        |CLOSED
   Fixed In Version|                            |sssd-2.5.0-2.fc35
         Resolution|---                         |CURRENTRELEASE
        Last Closed|                            |2021-06-08 11:08:10