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.

Sign up for GitHub

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

Fixes [#3172] possible position leak when location is disabled #3215

Closed
wants to merge 2 commits into from
Closed

Fixes [#3172] possible position leak when location is disabled #3215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7c91fa9
Fixes possible position leak when location is disabled
Gabrielerusso Feb 9, 2024
f8073d8
Merge branch 'master' into Fix-position-data-leaks
thebentern Feb 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 12 additions & 15 deletions src/GPSStatus.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,10 @@ class GPSStatus : public Status
#endif
meshtastic_NodeInfoLite *node = nodeDB.getMeshNode(nodeDB.getNodeNum());
return node->position.latitude_i;
} else {
} else if (config.position.gps_mode == meshtastic_Config_PositionConfig_GpsMode_ENABLED) {
return p.latitude_i;
} else {
return 0;
}
}

Expand All @@ -70,8 +72,10 @@ class GPSStatus : public Status
#endif
meshtastic_NodeInfoLite *node = nodeDB.getMeshNode(nodeDB.getNodeNum());
return node->position.longitude_i;
} else {
} else if (config.position.gps_mode == meshtastic_Config_PositionConfig_GpsMode_ENABLED) {
return p.longitude_i;
} else {
return 0;
}
}

Expand All @@ -83,25 +87,18 @@ class GPSStatus : public Status
#endif
meshtastic_NodeInfoLite *node = nodeDB.getMeshNode(nodeDB.getNodeNum());
return node->position.altitude;
} else {
} else if (config.position.gps_mode == meshtastic_Config_PositionConfig_GpsMode_ENABLED) {
return p.altitude;
} else {
return 0;
}
}

uint32_t getDOP() const
{
return p.PDOP;
}
uint32_t getDOP() const { return p.PDOP; }

uint32_t getHeading() const
{
return p.ground_track;
}
uint32_t getHeading() const { return p.ground_track; }

uint32_t getNumSatellites() const
{
return p.sats_in_view;
}
uint32_t getNumSatellites() const { return p.sats_in_view; }

bool matches(const GPSStatus *newStatus) const
{
Expand Down
6 changes: 5 additions & 1 deletion src/gps/GPS.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1301,6 +1301,7 @@ bool GPS::whileIdle()
}
return isValid;
}

void GPS::enable()
{
enabled = true;
Expand All @@ -1313,7 +1314,10 @@ int32_t GPS::disable()
enabled = false;
setInterval(INT32_MAX);
setAwake(false);

// reset position data if location is disabled and not fixed
if (config.position.gps_mode == meshtastic_Config_PositionConfig_GpsMode_DISABLED && !config.position.fixed_position) {
positionModule->clearLastPosition();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this one is still valid even if the "gpsmode" is used for the hardware GNSS module, as when it's decativated there is no use for the previous data if "fixed position" is disabled.

Copy link
Member

Choose a reason for hiding this comment

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

I would prefer a clean pull, probably doing this on nodedb reset. @jp-bennett I think this could be related to the preventing a node from deleting itself from the nodedb, should we clear the last position from the current node when we reset the nodedb?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this could be related to the preventing a node from deleting itself from the nodedb

Does that change apply when doing a NodeDB reset? A full reset should probably delete the local node's entry, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does that change apply when doing a NodeDB reset?

The change i did applies only when GPS::disable() is called.

}
return INT32_MAX;
}

Expand Down
12 changes: 12 additions & 0 deletions src/modules/PositionModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,18 @@ PositionModule::PositionModule()
}
}

void PositionModule::clearLastPosition()
{
LOG_DEBUG("Clearing last position saved\n");
meshtastic_NodeInfoLite *node = nodeDB.getMeshNode(nodeDB.getNodeNum());
node->position.latitude_i = 0;
node->position.longitude_i = 0;
node->position.altitude = 0;
node->position.time = 0;
lastGpsLatitude = 0;
lastGpsLongitude = 0;
}

void PositionModule::clearPosition()
{
LOG_DEBUG("Clearing position on startup for sleepy tracker (ー。ー) zzz\n");
Expand Down
2 changes: 2 additions & 0 deletions src/modules/PositionModule.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ class PositionModule : public ProtobufModule<meshtastic_Position>, private concu

void handleNewPosition();

void clearLastPosition();

protected:
/** Called to handle a particular incoming message

Expand Down
Loading