Crash in pl_Def* with Browsec VPN addon

NEW
Unassigned

Status

--
critical
2 years ago
3 months ago

People

(Reporter: lovemetal_vgf, Unassigned, NeedInfo)

Tracking

4.12
x86
Windows 7

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

(Reporter)

Description

2 years ago
Can you help me with this annoying crash please?

This bug was filed from the Socorro interface and is 
report bp-722c5678-da14-43a3-be5a-61d6a2160515.
=============================================================

Thank you.

Comment 1

2 years ago
Do you have steps to reproduce? Did you test in safe mode and with a fresh profile?
Flags: needinfo?(lovemetal_vgf)
I see about 550 null deref crashes in the past 7 days in the following pl_Def* functions: pl_DefSend, pl_DefGetsocketoption, pl_DefGetsockname:

https://crash-stats.mozilla.com/search/?signature=~pl_Def&address=%3D0x0&_facets=signature&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform#facet-signature

They occur in versions between FF29 and FF49.

I took a look at the minidump for one report (https://crash-stats.mozilla.com/report/index/d1dfbe93-3007-4f95-8c67-313912160522). It said that |fd->lower| was nullptr, which is not supposed to happen -- there is a |PR_ASSERT(fd->lower != NULL);| immediately before the crash point.
Crash Signature: [@ pl_DefSend] → [@ pl_DefSend] [@ pl_DefGetsocketoption ] [@ pl_DefGetsockname ]
Component: General → General
Product: Firefox → Core
Summary: Crash in pl_DefSend → Crash in pl_Def*
Assignee: nobody → nobody
Component: General → Libraries
Product: Core → NSS
Version: 46 Branch → trunk
Kai, any ideas?
Flags: needinfo?(kaie)
(Reporter)

Comment 4

2 years ago
UPDATE: I deactivated the add-on Browsec and no more crashes.
(In reply to Nicholas Nethercote [:njn] from comment #2)
> I see about 550 null deref crashes in the past 7 days in the following
> pl_Def* functions: pl_DefSend, pl_DefGetsocketoption, pl_DefGetsockname:
> 
> It said that |fd->lower| was nullptr, which is not supposed
> to happen -- there is a |PR_ASSERT(fd->lower != NULL);| immediately before
> the crash point.

PR_ASSERT has no effect in opt builds. All those functions (pl_Def*) have to check the arguments they're using for nullptr.
Assignee: nobody → nobody
Component: Libraries → NSPR
Product: NSS → NSPR
Version: trunk → 4.12
> PR_ASSERT has no effect in opt builds. All those functions (pl_Def*) have to
> check the arguments they're using for nullptr.

The PR_ASSERT suggests that a null fd->lower is expected to be impossible, which suggests that there's a bug elsewhere. So the fix is, to me, unclear. We could just skip the call if fd->lower is null and return something (not sure what), but maybe that's no better than crashing?

Comment 7

2 years ago
(In reply to Nicholas Nethercote [:njn] from comment #2)
> I took a look at the minidump for one report
> (https://crash-stats.mozilla.com/report/index/d1dfbe93-3007-4f95-8c67-
> 313912160522). It said that |fd->lower| was nullptr,

For my education, where on that page do you see that fd->lower was NULL ?
Flags: needinfo?(kaie)

Updated

2 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 8

2 years ago
(In reply to Nicholas Nethercote [:njn] from comment #6)
> We could just skip the call if fd->lower is null and return something (not
> sure what), but maybe that's no better than crashing?

I think we shouldn't crash in this scenario. If fd->lower is null, I think we're already in a corrupted state and shouldn't proceed further.

If fd->lower is null, we have:
- either memory corruption
- or some sort of race, where a socket has already been closed/destroyed, but another thread is still operating on it.
> > I took a look at the minidump for one report
> > (https://crash-stats.mozilla.com/report/index/d1dfbe93-3007-4f95-8c67-
> > 313912160522). It said that |fd->lower| was nullptr,
> 
> For my education, where on that page do you see that fd->lower was NULL ?

It's not on that page. You have to download the minidump -- which requires minidump access privileges, which not everybody has -- and open it in MSVC. See https://developer.mozilla.org/en-US/docs/Mozilla/Debugging/Debugging_a_minidump for more details.
(100.0% in signature vs 00.30% overall) Addon "Browsec VPN" = true
Summary: Crash in pl_Def* → Crash in pl_Def* with Browsec VPN addon

Updated

3 months ago
Crash Signature: [@ pl_DefSend] [@ pl_DefGetsocketoption ] [@ pl_DefGetsockname ] → [@ pl_DefSend] [@ pl_DefGetsocketoption ] [@ pl_DefGetsockname ] [@ static PRStatus pl_DefGetsockname]
You need to log in before you can comment on or make changes to this bug.