Re: [PATCH 05/18] iio: change strncpy+truncation to strlcpy
On Mon, 16 Jul 2018 13:42:06 +0200 Dominique Martinet wrote: > Jonathan Cameron wrote on Sun, Jul 15, 2018: > > On Fri, 13 Jul 2018 03:25:34 +0200 > > Dominique Martinet wrote: > > > Generated by scripts/coccinelle/misc/strncpy_truncation.cocci > > > > > > Signed-off-by: Dominique Martinet > > > > Applied to the togreg branch of iio.git and pushed out as testing > > for the autobuilders to play with it. > > Thanks! > > I have been pointed out that strlcpy, unlike strncpy, will read past the > size given in the input string and thus is Bad™ if the input string is > not nul terminated. > > After taking the time to check I believe this should not happen as the > original name seems to come from a dentry's d_name after proper > preparation (a buffer is allocated precisely for this purpose), but it > will not hurt to wait for that version. > > > The second reason I was waiting is that I intended to check for each > patch if it is safe to not pad the end of the string with zeroes (to > avoid e.g. information leaks) and that seems OK as well here after a > quick check but I wouldn't trust my own eyes this late so I'll let you > be judge of that if you feel like taking v1 anyway. > > Otherwise, I'll recheck properly and submit a v2 with strscpy and a > better commit message after the coccinelle script is taken for inclusion > and doing a better check but this might take a while longer. > > > Thanks, In this particular case I'm fairly sure it is safe so I'll leave it as is. Thanks, Jonathan
Re: [PATCH 05/18] iio: change strncpy+truncation to strlcpy
On Mon, 16 Jul 2018 13:42:06 +0200 Dominique Martinet wrote: > Jonathan Cameron wrote on Sun, Jul 15, 2018: > > On Fri, 13 Jul 2018 03:25:34 +0200 > > Dominique Martinet wrote: > > > Generated by scripts/coccinelle/misc/strncpy_truncation.cocci > > > > > > Signed-off-by: Dominique Martinet > > > > Applied to the togreg branch of iio.git and pushed out as testing > > for the autobuilders to play with it. > > Thanks! > > I have been pointed out that strlcpy, unlike strncpy, will read past the > size given in the input string and thus is Bad™ if the input string is > not nul terminated. > > After taking the time to check I believe this should not happen as the > original name seems to come from a dentry's d_name after proper > preparation (a buffer is allocated precisely for this purpose), but it > will not hurt to wait for that version. > > > The second reason I was waiting is that I intended to check for each > patch if it is safe to not pad the end of the string with zeroes (to > avoid e.g. information leaks) and that seems OK as well here after a > quick check but I wouldn't trust my own eyes this late so I'll let you > be judge of that if you feel like taking v1 anyway. > > Otherwise, I'll recheck properly and submit a v2 with strscpy and a > better commit message after the coccinelle script is taken for inclusion > and doing a better check but this might take a while longer. > > > Thanks, In this particular case I'm fairly sure it is safe so I'll leave it as is. Thanks, Jonathan
Re: [PATCH 05/18] iio: change strncpy+truncation to strlcpy
Jonathan Cameron wrote on Sun, Jul 15, 2018: > On Fri, 13 Jul 2018 03:25:34 +0200 > Dominique Martinet wrote: > > Generated by scripts/coccinelle/misc/strncpy_truncation.cocci > > > > Signed-off-by: Dominique Martinet > > Applied to the togreg branch of iio.git and pushed out as testing > for the autobuilders to play with it. Thanks! I have been pointed out that strlcpy, unlike strncpy, will read past the size given in the input string and thus is Bad™ if the input string is not nul terminated. After taking the time to check I believe this should not happen as the original name seems to come from a dentry's d_name after proper preparation (a buffer is allocated precisely for this purpose), but it will not hurt to wait for that version. The second reason I was waiting is that I intended to check for each patch if it is safe to not pad the end of the string with zeroes (to avoid e.g. information leaks) and that seems OK as well here after a quick check but I wouldn't trust my own eyes this late so I'll let you be judge of that if you feel like taking v1 anyway. Otherwise, I'll recheck properly and submit a v2 with strscpy and a better commit message after the coccinelle script is taken for inclusion and doing a better check but this might take a while longer. Thanks, -- Dominique Martinet
Re: [PATCH 05/18] iio: change strncpy+truncation to strlcpy
Jonathan Cameron wrote on Sun, Jul 15, 2018: > On Fri, 13 Jul 2018 03:25:34 +0200 > Dominique Martinet wrote: > > Generated by scripts/coccinelle/misc/strncpy_truncation.cocci > > > > Signed-off-by: Dominique Martinet > > Applied to the togreg branch of iio.git and pushed out as testing > for the autobuilders to play with it. Thanks! I have been pointed out that strlcpy, unlike strncpy, will read past the size given in the input string and thus is Bad™ if the input string is not nul terminated. After taking the time to check I believe this should not happen as the original name seems to come from a dentry's d_name after proper preparation (a buffer is allocated precisely for this purpose), but it will not hurt to wait for that version. The second reason I was waiting is that I intended to check for each patch if it is safe to not pad the end of the string with zeroes (to avoid e.g. information leaks) and that seems OK as well here after a quick check but I wouldn't trust my own eyes this late so I'll let you be judge of that if you feel like taking v1 anyway. Otherwise, I'll recheck properly and submit a v2 with strscpy and a better commit message after the coccinelle script is taken for inclusion and doing a better check but this might take a while longer. Thanks, -- Dominique Martinet
Re: [PATCH 05/18] iio: change strncpy+truncation to strlcpy
On Fri, 13 Jul 2018 03:25:34 +0200 Dominique Martinet wrote: > Generated by scripts/coccinelle/misc/strncpy_truncation.cocci > > Signed-off-by: Dominique Martinet Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. Thanks, Jonathan > --- > > Please see https://marc.info/?l=linux-kernel=153144450722324=2 (the > first patch of the serie) for the motivation behind this patch > > drivers/iio/common/st_sensors/st_sensors_core.c | 3 +-- > drivers/iio/pressure/st_pressure_i2c.c | 3 +-- > 2 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c > b/drivers/iio/common/st_sensors/st_sensors_core.c > index 57db19182e95..26fbd1bd9413 100644 > --- a/drivers/iio/common/st_sensors/st_sensors_core.c > +++ b/drivers/iio/common/st_sensors/st_sensors_core.c > @@ -380,8 +380,7 @@ void st_sensors_of_name_probe(struct device *dev, > return; > > /* The name from the OF match takes precedence if present */ > - strncpy(name, of_id->data, len); > - name[len - 1] = '\0'; > + strlcpy(name, of_id->data, len); > } > EXPORT_SYMBOL(st_sensors_of_name_probe); > #else > diff --git a/drivers/iio/pressure/st_pressure_i2c.c > b/drivers/iio/pressure/st_pressure_i2c.c > index fbb59059e942..2026a1012012 100644 > --- a/drivers/iio/pressure/st_pressure_i2c.c > +++ b/drivers/iio/pressure/st_pressure_i2c.c > @@ -94,9 +94,8 @@ static int st_press_i2c_probe(struct i2c_client *client, > if ((ret < 0) || (ret >= ST_PRESS_MAX)) > return -ENODEV; > > - strncpy(client->name, st_press_id_table[ret].name, > + strlcpy(client->name, st_press_id_table[ret].name, > sizeof(client->name)); > - client->name[sizeof(client->name) - 1] = '\0'; > } else if (!id) > return -ENODEV; >
Re: [PATCH 05/18] iio: change strncpy+truncation to strlcpy
On Fri, 13 Jul 2018 03:25:34 +0200 Dominique Martinet wrote: > Generated by scripts/coccinelle/misc/strncpy_truncation.cocci > > Signed-off-by: Dominique Martinet Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. Thanks, Jonathan > --- > > Please see https://marc.info/?l=linux-kernel=153144450722324=2 (the > first patch of the serie) for the motivation behind this patch > > drivers/iio/common/st_sensors/st_sensors_core.c | 3 +-- > drivers/iio/pressure/st_pressure_i2c.c | 3 +-- > 2 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c > b/drivers/iio/common/st_sensors/st_sensors_core.c > index 57db19182e95..26fbd1bd9413 100644 > --- a/drivers/iio/common/st_sensors/st_sensors_core.c > +++ b/drivers/iio/common/st_sensors/st_sensors_core.c > @@ -380,8 +380,7 @@ void st_sensors_of_name_probe(struct device *dev, > return; > > /* The name from the OF match takes precedence if present */ > - strncpy(name, of_id->data, len); > - name[len - 1] = '\0'; > + strlcpy(name, of_id->data, len); > } > EXPORT_SYMBOL(st_sensors_of_name_probe); > #else > diff --git a/drivers/iio/pressure/st_pressure_i2c.c > b/drivers/iio/pressure/st_pressure_i2c.c > index fbb59059e942..2026a1012012 100644 > --- a/drivers/iio/pressure/st_pressure_i2c.c > +++ b/drivers/iio/pressure/st_pressure_i2c.c > @@ -94,9 +94,8 @@ static int st_press_i2c_probe(struct i2c_client *client, > if ((ret < 0) || (ret >= ST_PRESS_MAX)) > return -ENODEV; > > - strncpy(client->name, st_press_id_table[ret].name, > + strlcpy(client->name, st_press_id_table[ret].name, > sizeof(client->name)); > - client->name[sizeof(client->name) - 1] = '\0'; > } else if (!id) > return -ENODEV; >
[PATCH 05/18] iio: change strncpy+truncation to strlcpy
Generated by scripts/coccinelle/misc/strncpy_truncation.cocci Signed-off-by: Dominique Martinet --- Please see https://marc.info/?l=linux-kernel=153144450722324=2 (the first patch of the serie) for the motivation behind this patch drivers/iio/common/st_sensors/st_sensors_core.c | 3 +-- drivers/iio/pressure/st_pressure_i2c.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c index 57db19182e95..26fbd1bd9413 100644 --- a/drivers/iio/common/st_sensors/st_sensors_core.c +++ b/drivers/iio/common/st_sensors/st_sensors_core.c @@ -380,8 +380,7 @@ void st_sensors_of_name_probe(struct device *dev, return; /* The name from the OF match takes precedence if present */ - strncpy(name, of_id->data, len); - name[len - 1] = '\0'; + strlcpy(name, of_id->data, len); } EXPORT_SYMBOL(st_sensors_of_name_probe); #else diff --git a/drivers/iio/pressure/st_pressure_i2c.c b/drivers/iio/pressure/st_pressure_i2c.c index fbb59059e942..2026a1012012 100644 --- a/drivers/iio/pressure/st_pressure_i2c.c +++ b/drivers/iio/pressure/st_pressure_i2c.c @@ -94,9 +94,8 @@ static int st_press_i2c_probe(struct i2c_client *client, if ((ret < 0) || (ret >= ST_PRESS_MAX)) return -ENODEV; - strncpy(client->name, st_press_id_table[ret].name, + strlcpy(client->name, st_press_id_table[ret].name, sizeof(client->name)); - client->name[sizeof(client->name) - 1] = '\0'; } else if (!id) return -ENODEV; -- 2.17.1
[PATCH 05/18] iio: change strncpy+truncation to strlcpy
Generated by scripts/coccinelle/misc/strncpy_truncation.cocci Signed-off-by: Dominique Martinet --- Please see https://marc.info/?l=linux-kernel=153144450722324=2 (the first patch of the serie) for the motivation behind this patch drivers/iio/common/st_sensors/st_sensors_core.c | 3 +-- drivers/iio/pressure/st_pressure_i2c.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/iio/common/st_sensors/st_sensors_core.c b/drivers/iio/common/st_sensors/st_sensors_core.c index 57db19182e95..26fbd1bd9413 100644 --- a/drivers/iio/common/st_sensors/st_sensors_core.c +++ b/drivers/iio/common/st_sensors/st_sensors_core.c @@ -380,8 +380,7 @@ void st_sensors_of_name_probe(struct device *dev, return; /* The name from the OF match takes precedence if present */ - strncpy(name, of_id->data, len); - name[len - 1] = '\0'; + strlcpy(name, of_id->data, len); } EXPORT_SYMBOL(st_sensors_of_name_probe); #else diff --git a/drivers/iio/pressure/st_pressure_i2c.c b/drivers/iio/pressure/st_pressure_i2c.c index fbb59059e942..2026a1012012 100644 --- a/drivers/iio/pressure/st_pressure_i2c.c +++ b/drivers/iio/pressure/st_pressure_i2c.c @@ -94,9 +94,8 @@ static int st_press_i2c_probe(struct i2c_client *client, if ((ret < 0) || (ret >= ST_PRESS_MAX)) return -ENODEV; - strncpy(client->name, st_press_id_table[ret].name, + strlcpy(client->name, st_press_id_table[ret].name, sizeof(client->name)); - client->name[sizeof(client->name) - 1] = '\0'; } else if (!id) return -ENODEV; -- 2.17.1
