[RFC PATCH v2 0/7] Fix year 2038 issue for sound subsystem

[RFC PATCH v2 0/7] Fix year 2038 issue for sound subsystem

[Baolin Wang]

Since many structures will use timespec type variables to record time stamp
in uapi/asound.h, which are not year 2038 safe on 32bit system. This patchset
tries to introduce new structures removing timespec type to compatible native
mode and compat mode.

Moreover this patchset also converts the internal structrures to use timespec64
type and related APIs.

Changes since v1:
 - Flat all structues to make them more clear.
 - Re-modify the convertion for struct snd_timer_tread.
 - Use same coding style when converting struct snd_rawmidi_status.
 - Use struct snd_timer_user_status64 in compat mode directly when converting
 struct snd_timer_status.
 - Add #ifdef __KERNEL__ when converting struct snd_ctl_elem_value.
 - Re-modify the convertion for struct snd_pcm_sync_ptr.
 - Fix some building errors.

Appreciated for any comments, especially for patch6 which I am not sure if it
is correct.

Baolin Wang (7):
  sound: Replace timespec with timespec64
  sound: core: Avoid using timespec for struct snd_pcm_status
  sound: core: Avoid using timespec for struct snd_rawmidi_status
  sound: core: Avoid using timespec for struct snd_timer_status
  uapi: sound: Avoid using timespec for struct snd_ctl_elem_value
  sound: core: Avoid using timespec for struct snd_pcm_sync_ptr
  sound: core: Avoid using timespec for struct snd_timer_tread

 include/sound/pcm.h               |  126 +++++++++-
 include/sound/timer.h             |    4 +-
 include/uapi/sound/asound.h       |   19 +-
 sound/core/pcm.c                  |   20 +-
 sound/core/pcm_compat.c           |  469 ++++++++++++++++++++++++++++---------
 sound/core/pcm_lib.c              |   33 ++-
 sound/core/pcm_native.c           |  220 +++++++++++++----
 sound/core/rawmidi.c              |   81 ++++++-
 sound/core/rawmidi_compat.c       |  121 +++++++---
 sound/core/timer.c                |  253 ++++++++++++++++----
 sound/core/timer_compat.c         |   67 ++----
 sound/pci/hda/hda_controller.c    |   10 +-
 sound/soc/intel/skylake/skl-pcm.c |    4 +-
 13 files changed, 1090 insertions(+), 337 deletions(-)

-- 
1.7.9.5

[RFC PATCH v2 1/7] sound: Replace timespec with timespec64

[Baolin Wang]

Since timespec is not year 2038 safe on 32bit system, and we need to
convert all timespec variables to timespec64 type for sound subsystem.

This patch is used to do preparation for following patches, that will
convert all structures defined in uapi/sound/asound.h to use 64-bit
time_t.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 include/sound/pcm.h               |   18 +++++++++---------
 include/sound/timer.h             |    4 ++--
 sound/core/pcm_lib.c              |   30 +++++++++++++++++-------------
 sound/core/pcm_native.c           |   12 ++++++++----
 sound/core/timer.c                |   28 ++++++++++++++--------------
 sound/pci/hda/hda_controller.c    |   10 +++++-----
 sound/soc/intel/skylake/skl-pcm.c |    4 ++--
 7 files changed, 57 insertions(+), 49 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 24febf9..cd1ecd6 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -75,7 +75,7 @@ struct snd_pcm_ops {
 	int (*trigger)(struct snd_pcm_substream *substream, int cmd);
 	snd_pcm_uframes_t (*pointer)(struct snd_pcm_substream *substream);
 	int (*get_time_info)(struct snd_pcm_substream *substream,
-			struct timespec *system_ts, struct timespec *audio_ts,
+			struct timespec64 *system_ts, struct timespec64 *audio_ts,
 			struct snd_pcm_audio_tstamp_config *audio_tstamp_config,
 			struct snd_pcm_audio_tstamp_report *audio_tstamp_report);
 	int (*fill_silence)(struct snd_pcm_substream *substream, int channel,
@@ -343,7 +343,7 @@ static inline void snd_pcm_pack_audio_tstamp_report(__u32 *data, __u32 *accuracy
 struct snd_pcm_runtime {
 	/* -- Status -- */
 	struct snd_pcm_substream *trigger_master;
-	struct timespec trigger_tstamp;	/* trigger timestamp */
+	struct timespec64 trigger_tstamp;	/* trigger timestamp */
 	bool trigger_tstamp_latched;     /* trigger timestamp latched in low-level driver/hardware */
 	int overrange;
 	snd_pcm_uframes_t avail_max;
@@ -419,7 +419,7 @@ struct snd_pcm_runtime {
 	/* -- audio timestamp config -- */
 	struct snd_pcm_audio_tstamp_config audio_tstamp_config;
 	struct snd_pcm_audio_tstamp_report audio_tstamp_report;
-	struct timespec driver_tstamp;
+	struct timespec64 driver_tstamp;
 
 #if IS_ENABLED(CONFIG_SND_PCM_OSS)
 	/* -- OSS things -- */
@@ -1167,22 +1167,22 @@ static inline void snd_pcm_set_runtime_buffer(struct snd_pcm_substream *substrea
 }
 
 /**
- * snd_pcm_gettime - Fill the timespec depending on the timestamp mode
+ * snd_pcm_gettime - Fill the timespec64 depending on the timestamp mode
  * @runtime: PCM runtime instance
- * @tv: timespec to fill
+ * @tv: timespec64 to fill
  */
 static inline void snd_pcm_gettime(struct snd_pcm_runtime *runtime,
-				   struct timespec *tv)
+				   struct timespec64 *tv)
 {
 	switch (runtime->tstamp_type) {
 	case SNDRV_PCM_TSTAMP_TYPE_MONOTONIC:
-		ktime_get_ts(tv);
+		ktime_get_ts64(tv);
 		break;
 	case SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW:
-		getrawmonotonic(tv);
+		getrawmonotonic64(tv);
 		break;
 	default:
-		getnstimeofday(tv);
+		ktime_get_real_ts64(tv);
 		break;
 	}
 }
diff --git a/include/sound/timer.h b/include/sound/timer.h
index c4d76ff..c196c07 100644
--- a/include/sound/timer.h
+++ b/include/sound/timer.h
@@ -102,7 +102,7 @@ struct snd_timer_instance {
 			  unsigned long ticks, unsigned long resolution);
 	void (*ccallback) (struct snd_timer_instance * timeri,
 			   int event,
-			   struct timespec * tstamp,
+			   struct timespec64 * tstamp,
 			   unsigned long resolution);
 	void (*disconnect)(struct snd_timer_instance *timeri);
 	void *callback_data;
@@ -126,7 +126,7 @@ struct snd_timer_instance {
  */
 
 int snd_timer_new(struct snd_card *card, char *id, struct snd_timer_id *tid, struct snd_timer **rtimer);
-void snd_timer_notify(struct snd_timer *timer, int event, struct timespec *tstamp);
+void snd_timer_notify(struct snd_timer *timer, int event, struct timespec64 *tstamp);
 int snd_timer_global_new(char *id, int device, struct snd_timer **rtimer);
 int snd_timer_global_free(struct snd_timer *timer);
 int snd_timer_global_register(struct snd_timer *timer);
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index a93a423..5ca9dc3 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -158,8 +158,12 @@ static void xrun(struct snd_pcm_substream *substream)
 	struct snd_pcm_runtime *runtime = substream->runtime;
 
 	trace_xrun(substream);
-	if (runtime->tstamp_mode == SNDRV_PCM_TSTAMP_ENABLE)
-		snd_pcm_gettime(runtime, (struct timespec *)&runtime->status->tstamp);
+	if (runtime->tstamp_mode == SNDRV_PCM_TSTAMP_ENABLE) {
+		struct timespec64 tstamp;
+
+		snd_pcm_gettime(runtime, &tstamp);
+		runtime->status->tstamp = timespec64_to_timespec(tstamp);
+	}
 	snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN);
 	if (xrun_debug(substream, XRUN_DEBUG_BASIC)) {
 		char name[16];
@@ -217,12 +221,12 @@ int snd_pcm_update_state(struct snd_pcm_substream *substream,
 }
 
 static void update_audio_tstamp(struct snd_pcm_substream *substream,
-				struct timespec *curr_tstamp,
-				struct timespec *audio_tstamp)
+				struct timespec64 *curr_tstamp,
+				struct timespec64 *audio_tstamp)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	u64 audio_frames, audio_nsecs;
-	struct timespec driver_tstamp;
+	struct timespec64 driver_tstamp;
 
 	if (runtime->tstamp_mode != SNDRV_PCM_TSTAMP_ENABLE)
 		return;
@@ -246,16 +250,16 @@ static void update_audio_tstamp(struct snd_pcm_substream *substream,
 		}
 		audio_nsecs = div_u64(audio_frames * 1000000000LL,
 				runtime->rate);
-		*audio_tstamp = ns_to_timespec(audio_nsecs);
+		*audio_tstamp = ns_to_timespec64(audio_nsecs);
 	}
-	runtime->status->audio_tstamp = *audio_tstamp;
-	runtime->status->tstamp = *curr_tstamp;
+	runtime->status->audio_tstamp = timespec64_to_timespec(*audio_tstamp);
+	runtime->status->tstamp = timespec64_to_timespec(*curr_tstamp);
 
 	/*
 	 * re-take a driver timestamp to let apps detect if the reference tstamp
 	 * read by low-level hardware was provided with a delay
 	 */
-	snd_pcm_gettime(substream->runtime, (struct timespec *)&driver_tstamp);
+	snd_pcm_gettime(substream->runtime, &driver_tstamp);
 	runtime->driver_tstamp = driver_tstamp;
 }
 
@@ -268,8 +272,8 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
 	snd_pcm_sframes_t hdelta, delta;
 	unsigned long jdelta;
 	unsigned long curr_jiffies;
-	struct timespec curr_tstamp;
-	struct timespec audio_tstamp;
+	struct timespec64 curr_tstamp;
+	struct timespec64 audio_tstamp;
 	int crossed_boundary = 0;
 
 	old_hw_ptr = runtime->status->hw_ptr;
@@ -292,9 +296,9 @@ static int snd_pcm_update_hw_ptr0(struct snd_pcm_substream *substream,
 
 			/* re-test in case tstamp type is not supported in hardware and was demoted to DEFAULT */
 			if (runtime->audio_tstamp_report.actual_type == SNDRV_PCM_AUDIO_TSTAMP_TYPE_DEFAULT)
-				snd_pcm_gettime(runtime, (struct timespec *)&curr_tstamp);
+				snd_pcm_gettime(runtime, &curr_tstamp);
 		} else
-			snd_pcm_gettime(runtime, (struct timespec *)&curr_tstamp);
+			snd_pcm_gettime(runtime, &curr_tstamp);
 	}
 
 	if (pos == SNDRV_PCM_POS_XRUN) {
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 2fec2fe..60bc303 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -881,12 +881,12 @@ int snd_pcm_status(struct snd_pcm_substream *substream,
 	status->suspended_state = runtime->status->suspended_state;
 	if (status->state == SNDRV_PCM_STATE_OPEN)
 		goto _end;
-	status->trigger_tstamp = runtime->trigger_tstamp;
+	status->trigger_tstamp = timespec64_to_timespec(runtime->trigger_tstamp);
 	if (snd_pcm_running(substream)) {
 		snd_pcm_update_hw_ptr(substream);
 		if (runtime->tstamp_mode == SNDRV_PCM_TSTAMP_ENABLE) {
 			status->tstamp = runtime->status->tstamp;
-			status->driver_tstamp = runtime->driver_tstamp;
+			status->driver_tstamp = timespec64_to_timespec(runtime->driver_tstamp);
 			status->audio_tstamp =
 				runtime->status->audio_tstamp;
 			if (runtime->audio_tstamp_report.valid == 1)
@@ -899,8 +899,12 @@ int snd_pcm_status(struct snd_pcm_substream *substream,
 		}
 	} else {
 		/* get tstamp only in fallback mode and only if enabled */
-		if (runtime->tstamp_mode == SNDRV_PCM_TSTAMP_ENABLE)
-			snd_pcm_gettime(runtime, &status->tstamp);
+		if (runtime->tstamp_mode == SNDRV_PCM_TSTAMP_ENABLE) {
+			struct timespec64 tstamp;
+
+			snd_pcm_gettime(runtime, &tstamp);
+			status->tstamp = timespec64_to_timespec(tstamp);
+		}
 	}
  _tstamp_end:
 	status->appl_ptr = runtime->control->appl_ptr;
diff --git a/sound/core/timer.c b/sound/core/timer.c
index 6cdd04a..f44d702 100644
--- a/sound/core/timer.c
+++ b/sound/core/timer.c
@@ -73,7 +73,7 @@ struct snd_timer_user {
 	spinlock_t qlock;
 	unsigned long last_resolution;
 	unsigned int filter;
-	struct timespec tstamp;		/* trigger tstamp */
+	struct timespec64 tstamp;		/* trigger tstamp */
 	wait_queue_head_t qchange_sleep;
 	struct fasync_struct *fasync;
 	struct mutex ioctl_lock;
@@ -408,12 +408,12 @@ static void snd_timer_notify1(struct snd_timer_instance *ti, int event)
 	struct snd_timer *timer;
 	unsigned long resolution = 0;
 	struct snd_timer_instance *ts;
-	struct timespec tstamp;
+	struct timespec64 tstamp;
 
 	if (timer_tstamp_monotonic)
-		ktime_get_ts(&tstamp);
+		ktime_get_ts64(&tstamp);
 	else
-		getnstimeofday(&tstamp);
+		ktime_get_real_ts64(&tstamp);
 	if (snd_BUG_ON(event < SNDRV_TIMER_EVENT_START ||
 		       event > SNDRV_TIMER_EVENT_PAUSE))
 		return;
@@ -957,7 +957,7 @@ static int snd_timer_dev_disconnect(struct snd_device *device)
 	return 0;
 }
 
-void snd_timer_notify(struct snd_timer *timer, int event, struct timespec *tstamp)
+void snd_timer_notify(struct snd_timer *timer, int event, struct timespec64 *tstamp)
 {
 	unsigned long flags;
 	unsigned long resolution = 0;
@@ -1251,7 +1251,7 @@ static void snd_timer_user_append_to_tqueue(struct snd_timer_user *tu,
 
 static void snd_timer_user_ccallback(struct snd_timer_instance *timeri,
 				     int event,
-				     struct timespec *tstamp,
+				     struct timespec64 *tstamp,
 				     unsigned long resolution)
 {
 	struct snd_timer_user *tu = timeri->callback_data;
@@ -1265,7 +1265,7 @@ static void snd_timer_user_ccallback(struct snd_timer_instance *timeri,
 		return;
 	memset(&r1, 0, sizeof(r1));
 	r1.event = event;
-	r1.tstamp = *tstamp;
+	r1.tstamp = timespec64_to_timespec(*tstamp);
 	r1.val = resolution;
 	spin_lock_irqsave(&tu->qlock, flags);
 	snd_timer_user_append_to_tqueue(tu, &r1);
@@ -1288,7 +1288,7 @@ static void snd_timer_user_tinterrupt(struct snd_timer_instance *timeri,
 {
 	struct snd_timer_user *tu = timeri->callback_data;
 	struct snd_timer_tread *r, r1;
-	struct timespec tstamp;
+	struct timespec64 tstamp;
 	int prev, append = 0;
 
 	memset(&r1, 0, sizeof(r1));
@@ -1301,14 +1301,14 @@ static void snd_timer_user_tinterrupt(struct snd_timer_instance *timeri,
 	}
 	if (tu->last_resolution != resolution || ticks > 0) {
 		if (timer_tstamp_monotonic)
-			ktime_get_ts(&tstamp);
+			ktime_get_ts64(&tstamp);
 		else
-			getnstimeofday(&tstamp);
+			ktime_get_real_ts64(&tstamp);
 	}
 	if ((tu->filter & (1 << SNDRV_TIMER_EVENT_RESOLUTION)) &&
 	    tu->last_resolution != resolution) {
 		r1.event = SNDRV_TIMER_EVENT_RESOLUTION;
-		r1.tstamp = tstamp;
+		r1.tstamp = timespec64_to_timespec(tstamp);
 		r1.val = resolution;
 		snd_timer_user_append_to_tqueue(tu, &r1);
 		tu->last_resolution = resolution;
@@ -1322,14 +1322,14 @@ static void snd_timer_user_tinterrupt(struct snd_timer_instance *timeri,
 		prev = tu->qtail == 0 ? tu->queue_size - 1 : tu->qtail - 1;
 		r = &tu->tqueue[prev];
 		if (r->event == SNDRV_TIMER_EVENT_TICK) {
-			r->tstamp = tstamp;
+			r->tstamp = timespec64_to_timespec(tstamp);
 			r->val += ticks;
 			append++;
 			goto __wake;
 		}
 	}
 	r1.event = SNDRV_TIMER_EVENT_TICK;
-	r1.tstamp = tstamp;
+	r1.tstamp = timespec64_to_timespec(tstamp);
 	r1.val = ticks;
 	snd_timer_user_append_to_tqueue(tu, &r1);
 	append++;
@@ -1808,7 +1808,7 @@ static int snd_timer_user_status(struct file *file,
 	if (!tu->timeri)
 		return -EBADFD;
 	memset(&status, 0, sizeof(status));
-	status.tstamp = tu->tstamp;
+	status.tstamp = timespec64_to_timespec(tu->tstamp);
 	status.resolution = snd_timer_resolution(tu->timeri);
 	status.lost = tu->timeri->lost;
 	status.overrun = tu->overrun;
diff --git a/sound/pci/hda/hda_controller.c b/sound/pci/hda/hda_controller.c
index d1eb148..c3e4516 100644
--- a/sound/pci/hda/hda_controller.c
+++ b/sound/pci/hda/hda_controller.c
@@ -502,7 +502,7 @@ static inline bool is_link_time_supported(struct snd_pcm_runtime *runtime,
 }
 
 static int azx_get_time_info(struct snd_pcm_substream *substream,
-			struct timespec *system_ts, struct timespec *audio_ts,
+			struct timespec64 *system_ts, struct timespec64 *audio_ts,
 			struct snd_pcm_audio_tstamp_config *audio_tstamp_config,
 			struct snd_pcm_audio_tstamp_report *audio_tstamp_report)
 {
@@ -522,7 +522,7 @@ static int azx_get_time_info(struct snd_pcm_substream *substream,
 		if (audio_tstamp_config->report_delay)
 			nsec = azx_adjust_codec_delay(substream, nsec);
 
-		*audio_ts = ns_to_timespec(nsec);
+		*audio_ts = ns_to_timespec64(nsec);
 
 		audio_tstamp_report->actual_type = SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK;
 		audio_tstamp_report->accuracy_report = 1; /* rest of structure is valid */
@@ -539,16 +539,16 @@ static int azx_get_time_info(struct snd_pcm_substream *substream,
 			return -EINVAL;
 
 		case SNDRV_PCM_TSTAMP_TYPE_MONOTONIC_RAW:
-			*system_ts = ktime_to_timespec(xtstamp.sys_monoraw);
+			*system_ts = ktime_to_timespec64(xtstamp.sys_monoraw);
 			break;
 
 		default:
-			*system_ts = ktime_to_timespec(xtstamp.sys_realtime);
+			*system_ts = ktime_to_timespec64(xtstamp.sys_realtime);
 			break;
 
 		}
 
-		*audio_ts = ktime_to_timespec(xtstamp.device);
+		*audio_ts = ktime_to_timespec64(xtstamp.device);
 
 		audio_tstamp_report->actual_type =
 			SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK_SYNCHRONIZED;
diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
index 2b1e513..9f905c4 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -1141,7 +1141,7 @@ static u64 skl_adjust_codec_delay(struct snd_pcm_substream *substream,
 }
 
 static int skl_get_time_info(struct snd_pcm_substream *substream,
-			struct timespec *system_ts, struct timespec *audio_ts,
+			struct timespec64 *system_ts, struct timespec64 *audio_ts,
 			struct snd_pcm_audio_tstamp_config *audio_tstamp_config,
 			struct snd_pcm_audio_tstamp_report *audio_tstamp_report)
 {
@@ -1159,7 +1159,7 @@ static int skl_get_time_info(struct snd_pcm_substream *substream,
 		if (audio_tstamp_config->report_delay)
 			nsec = skl_adjust_codec_delay(substream, nsec);
 
-		*audio_ts = ns_to_timespec(nsec);
+		*audio_ts = ns_to_timespec64(nsec);
 
 		audio_tstamp_report->actual_type = SNDRV_PCM_AUDIO_TSTAMP_TYPE_LINK;
 		audio_tstamp_report->accuracy_report = 1; /* rest of struct is valid */
-- 
1.7.9.5

[RFC PATCH v2 2/7] sound: core: Avoid using timespec for struct snd_pcm_status

[Baolin Wang]

The struct snd_pcm_status will use 'timespec' type variables to record
timestamp, which is not year 2038 safe on 32bits system.

Userspace will use SNDRV_PCM_IOCTL_STATUS and SNDRV_PCM_IOCTL_STATUS_EXT
as commands to issue ioctl() to fill the 'snd_pcm_status' structure in
userspace. The command number is always defined through _IOR/_IOW/IORW,
so when userspace changes the definition of 'struct timespec' to use
64-bit types, the command number also changes.

Thus in the kernel, we now need to define two versions of each such ioctl
and corresponding ioctl commands to handle 32bit time_t and 64bit time_t
in native mode:
struct snd_pcm_status32 {
	......
	struct { s32 tv_sec; s32 tv_nsec; } trigger_tstamp;
	struct { s32 tv_sec; s32 tv_nsec; } tstamp;
	......
}

struct snd_pcm_status64 {
	......
	struct { s64 tv_sec; s64 tv_nsec; } trigger_tstamp;
	struct { s64 tv_sec; s64 tv_nsec; } tstamp;
	......
}

Moreover in compat file, we renamed or introduced new structures to handle
32bit/64bit time_t in compatible mode. 'struct compat_snd_pcm_status32' and
snd_pcm_status_user_compat() are used to handle 32bit time_t in compat mode.
'struct compat_snd_pcm_status64' and snd_pcm_status_user_compat64() are used
to handle 64bit time_t with 64bit alignment. 'struct compat_snd_pcm_status64_x86_32'
and snd_pcm_status_user_compat64_x86_32() are used to handle 64bit time_t with
32bit alignment.

Finally we can replace SNDRV_PCM_IOCTL_STATUS and SNDRV_PCM_IOCTL_STATUS_EXT
with new commands and introduce new functions to fill new 'struct snd_pcm_status64'
instead of using unsafe 'struct snd_pcm_status'. Then in future, the new
commands can be matched when userspace changes 'timespec' to 64bit type
to make a size change of 'struct snd_pcm_status'. When glibc changes time_t
to 64-bit, any recompiled program will issue ioctl commands that the kernel
does not understand without this patch.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 include/sound/pcm.h     |   57 +++++++++++-
 sound/core/pcm.c        |   12 +--
 sound/core/pcm_compat.c |  236 +++++++++++++++++++++++++++++++++++------------
 sound/core/pcm_native.c |  100 ++++++++++++++++----
 4 files changed, 319 insertions(+), 86 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index cd1ecd6..7524b54 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -58,6 +58,7 @@ struct snd_pcm_hardware {
 	size_t fifo_size;		/* fifo size in bytes */
 };
 
+struct snd_pcm_status64;
 struct snd_pcm_substream;
 
 struct snd_pcm_audio_tstamp_config; /* definitions further down */
@@ -565,8 +566,8 @@ struct snd_pcm_notify {
 int snd_pcm_info(struct snd_pcm_substream *substream, struct snd_pcm_info *info);
 int snd_pcm_info_user(struct snd_pcm_substream *substream,
 		      struct snd_pcm_info __user *info);
-int snd_pcm_status(struct snd_pcm_substream *substream,
-		   struct snd_pcm_status *status);
+int snd_pcm_status64(struct snd_pcm_substream *substream,
+		     struct snd_pcm_status64 *status);
 int snd_pcm_start(struct snd_pcm_substream *substream);
 int snd_pcm_stop(struct snd_pcm_substream *substream, snd_pcm_state_t status);
 int snd_pcm_drain_done(struct snd_pcm_substream *substream);
@@ -1440,4 +1441,56 @@ static inline u64 pcm_format_to_bits(snd_pcm_format_t pcm_format)
 #define pcm_dbg(pcm, fmt, args...) \
 	dev_dbg((pcm)->card->dev, fmt, ##args)
 
+struct snd_pcm_status64 {
+	snd_pcm_state_t state;		/* stream state */
+	s64 trigger_tstamp_sec;		/* time when stream was started/stopped/paused */
+	s64 trigger_tstamp_nsec;
+	s64 tstamp_sec;			/* reference timestamp */
+	s64 tstamp_nsec;
+	snd_pcm_uframes_t appl_ptr;	/* appl ptr */
+	snd_pcm_uframes_t hw_ptr;	/* hw ptr */
+	snd_pcm_sframes_t delay;	/* current delay in frames */
+	snd_pcm_uframes_t avail;	/* number of frames available */
+	snd_pcm_uframes_t avail_max;	/* max frames available on hw since last status */
+	snd_pcm_uframes_t overrange;	/* count of ADC (capture) overrange detections from last status */
+	snd_pcm_state_t suspended_state; /* suspended stream state */
+	__u32 audio_tstamp_data;	 /* needed for 64-bit alignment, used for configs/report to/from userspace */
+	s64 audio_tstamp_sec;		/* sample counter, wall clock, PHC or on-demand sync'ed */
+	s64 audio_tstamp_nsec;
+	s64 driver_tstamp_sec;		/* useful in case reference system tstamp is reported with delay */
+	s64 driver_tstamp_nsec;
+	__u32 audio_tstamp_accuracy;	/* in ns units, only valid if indicated in audio_tstamp_data */
+	unsigned char reserved[52-4*sizeof(s64)]; /* must be filled with zero */
+};
+
+#define SNDRV_PCM_IOCTL_STATUS64	_IOR('A', 0x20, struct snd_pcm_status64)
+#define SNDRV_PCM_IOCTL_STATUS_EXT64	_IOWR('A', 0x24, struct snd_pcm_status64)
+
+#if __BITS_PER_LONG == 32
+struct snd_pcm_status32 {
+	snd_pcm_state_t state;		/* stream state */
+	s32 trigger_tstamp_sec;		/* time when stream was started/stopped/paused */
+	s32 trigger_tstamp_nsec;
+	s32 tstamp_sec;			/* reference timestamp */
+	s32 tstamp_nsec;
+	snd_pcm_uframes_t appl_ptr;	/* appl ptr */
+	snd_pcm_uframes_t hw_ptr;	/* hw ptr */
+	snd_pcm_sframes_t delay;	/* current delay in frames */
+	snd_pcm_uframes_t avail;	/* number of frames available */
+	snd_pcm_uframes_t avail_max;	/* max frames available on hw since last status */
+	snd_pcm_uframes_t overrange;	/* count of ADC (capture) overrange detections from last status */
+	snd_pcm_state_t suspended_state; /* suspended stream state */
+	__u32 audio_tstamp_data;	 /* needed for 64-bit alignment, used for configs/report to/from userspace */
+	s32 audio_tstamp_sec;		/* sample counter, wall clock, PHC or on-demand sync'ed */
+	s32 audio_tstamp_nsec;
+	s32 driver_tstamp_sec;		/* useful in case reference system tstamp is reported with delay */
+	s32 driver_tstamp_nsec;
+	__u32 audio_tstamp_accuracy;	/* in ns units, only valid if indicated in audio_tstamp_data */
+	unsigned char reserved[52-4*sizeof(s32)]; /* must be filled with zero */
+};
+
+#define SNDRV_PCM_IOCTL_STATUS32	_IOR('A', 0x20, struct snd_pcm_status32)
+#define SNDRV_PCM_IOCTL_STATUS_EXT32	_IOWR('A', 0x24, struct snd_pcm_status32)
+#endif
+
 #endif /* __SOUND_PCM_H */
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index 7eadb7f..c19e656 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -453,7 +453,7 @@ static void snd_pcm_substream_proc_status_read(struct snd_info_entry *entry,
 {
 	struct snd_pcm_substream *substream = entry->private_data;
 	struct snd_pcm_runtime *runtime;
-	struct snd_pcm_status status;
+	struct snd_pcm_status64 status;
 	int err;
 
 	mutex_lock(&substream->pcm->open_mutex);
@@ -463,17 +463,17 @@ static void snd_pcm_substream_proc_status_read(struct snd_info_entry *entry,
 		goto unlock;
 	}
 	memset(&status, 0, sizeof(status));
-	err = snd_pcm_status(substream, &status);
+	err = snd_pcm_status64(substream, &status);
 	if (err < 0) {
 		snd_iprintf(buffer, "error %d\n", err);
 		goto unlock;
 	}
 	snd_iprintf(buffer, "state: %s\n", snd_pcm_state_name(status.state));
 	snd_iprintf(buffer, "owner_pid   : %d\n", pid_vnr(substream->pid));
-	snd_iprintf(buffer, "trigger_time: %ld.%09ld\n",
-		status.trigger_tstamp.tv_sec, status.trigger_tstamp.tv_nsec);
-	snd_iprintf(buffer, "tstamp      : %ld.%09ld\n",
-		status.tstamp.tv_sec, status.tstamp.tv_nsec);
+	snd_iprintf(buffer, "trigger_time: %lld.%09lld\n",
+		status.trigger_tstamp_sec, status.trigger_tstamp_nsec);
+	snd_iprintf(buffer, "tstamp      : %lld.%09lld\n",
+		status.tstamp_sec, status.tstamp_nsec);
 	snd_iprintf(buffer, "delay       : %ld\n", status.delay);
 	snd_iprintf(buffer, "avail       : %ld\n", status.avail);
 	snd_iprintf(buffer, "avail_max   : %ld\n", status.avail_max);
diff --git a/sound/core/pcm_compat.c b/sound/core/pcm_compat.c
index b719d0b..53b83d4 100644
--- a/sound/core/pcm_compat.c
+++ b/sound/core/pcm_compat.c
@@ -187,10 +187,12 @@ static int snd_pcm_channel_info_user(struct snd_pcm_substream *substream,
 	snd_pcm_channel_info_user(s, p)
 #endif /* CONFIG_X86_X32 */
 
-struct snd_pcm_status32 {
+struct compat_snd_pcm_status32 {
 	s32 state;
-	struct compat_timespec trigger_tstamp;
-	struct compat_timespec tstamp;
+	s32 trigger_tstamp_sec;
+	s32 trigger_tstamp_nsec;
+	s32 tstamp_sec;
+	s32 tstamp_nsec;
 	u32 appl_ptr;
 	u32 hw_ptr;
 	s32 delay;
@@ -199,21 +201,25 @@ struct snd_pcm_status32 {
 	u32 overrange;
 	s32 suspended_state;
 	u32 audio_tstamp_data;
-	struct compat_timespec audio_tstamp;
-	struct compat_timespec driver_tstamp;
+	s32 audio_tstamp_sec;
+	s32 audio_tstamp_nsec;
+	s32 driver_tstamp_sec;
+	s32 driver_tstamp_nsec;
 	u32 audio_tstamp_accuracy;
-	unsigned char reserved[52-2*sizeof(struct compat_timespec)];
+	unsigned char reserved[52-4*sizeof(s32)];
 } __attribute__((packed));
 
 
 static int snd_pcm_status_user_compat(struct snd_pcm_substream *substream,
-				      struct snd_pcm_status32 __user *src,
+				      struct compat_snd_pcm_status32 __user *src,
 				      bool ext)
 {
-	struct snd_pcm_status status;
+	struct snd_pcm_status64 status;
+	struct compat_snd_pcm_status32 compat_status32;
 	int err;
 
 	memset(&status, 0, sizeof(status));
+	memset(&compat_status32, 0, sizeof(compat_status32));
 	/*
 	 * with extension, parameters are read/write,
 	 * get audio_tstamp_data from user,
@@ -222,38 +228,47 @@ static int snd_pcm_status_user_compat(struct snd_pcm_substream *substream,
 	if (ext && get_user(status.audio_tstamp_data,
 				(u32 __user *)(&src->audio_tstamp_data)))
 		return -EFAULT;
-	err = snd_pcm_status(substream, &status);
+	err = snd_pcm_status64(substream, &status);
 	if (err < 0)
 		return err;
 
 	if (clear_user(src, sizeof(*src)))
 		return -EFAULT;
-	if (put_user(status.state, &src->state) ||
-	    compat_put_timespec(&status.trigger_tstamp, &src->trigger_tstamp) ||
-	    compat_put_timespec(&status.tstamp, &src->tstamp) ||
-	    put_user(status.appl_ptr, &src->appl_ptr) ||
-	    put_user(status.hw_ptr, &src->hw_ptr) ||
-	    put_user(status.delay, &src->delay) ||
-	    put_user(status.avail, &src->avail) ||
-	    put_user(status.avail_max, &src->avail_max) ||
-	    put_user(status.overrange, &src->overrange) ||
-	    put_user(status.suspended_state, &src->suspended_state) ||
-	    put_user(status.audio_tstamp_data, &src->audio_tstamp_data) ||
-	    compat_put_timespec(&status.audio_tstamp, &src->audio_tstamp) ||
-	    compat_put_timespec(&status.driver_tstamp, &src->driver_tstamp) ||
-	    put_user(status.audio_tstamp_accuracy, &src->audio_tstamp_accuracy))
+
+	compat_status32 = (struct compat_snd_pcm_status32) {
+		.state = status.state,
+		.trigger_tstamp_sec = status.trigger_tstamp_sec,
+		.trigger_tstamp_nsec = status.trigger_tstamp_nsec,
+		.tstamp_sec = status.tstamp_sec,
+		.tstamp_nsec = status.tstamp_nsec,
+		.appl_ptr = status.appl_ptr,
+		.hw_ptr = status.hw_ptr,
+		.delay = status.delay,
+		.avail = status.avail,
+		.avail_max = status.avail_max,
+		.overrange = status.overrange,
+		.suspended_state = status.suspended_state,
+		.audio_tstamp_data = status.audio_tstamp_data,
+		.audio_tstamp_sec = status.audio_tstamp_sec,
+		.audio_tstamp_nsec = status.audio_tstamp_nsec,
+		.driver_tstamp_sec = status.audio_tstamp_sec,
+		.driver_tstamp_nsec = status.audio_tstamp_nsec,
+		.audio_tstamp_accuracy = status.audio_tstamp_accuracy,
+	};
+
+	if (copy_to_user(src, &compat_status32, sizeof(compat_status32)))
 		return -EFAULT;
 
 	return err;
 }
 
-#ifdef CONFIG_X86_X32
-/* X32 ABI has 64bit timespec and 64bit alignment */
-struct snd_pcm_status_x32 {
+struct compat_snd_pcm_status64 {
 	s32 state;
 	u32 rsvd; /* alignment */
-	struct timespec trigger_tstamp;
-	struct timespec tstamp;
+	s64 trigger_tstamp_sec;
+	s64 trigger_tstamp_nsec;
+	s64 tstamp_sec;
+	s64 tstamp_nsec;
 	u32 appl_ptr;
 	u32 hw_ptr;
 	s32 delay;
@@ -262,22 +277,26 @@ struct snd_pcm_status_x32 {
 	u32 overrange;
 	s32 suspended_state;
 	u32 audio_tstamp_data;
-	struct timespec audio_tstamp;
-	struct timespec driver_tstamp;
+	s64 audio_tstamp_sec;
+	s64 audio_tstamp_nsec;
+	s64 driver_tstamp_sec;
+	s64 driver_tstamp_nsec;
 	u32 audio_tstamp_accuracy;
-	unsigned char reserved[52-2*sizeof(struct timespec)];
+	unsigned char reserved[52-4*sizeof(s64)];
 } __packed;
 
 #define put_timespec(src, dst) copy_to_user(dst, src, sizeof(*dst))
 
-static int snd_pcm_status_user_x32(struct snd_pcm_substream *substream,
-				   struct snd_pcm_status_x32 __user *src,
-				   bool ext)
+static int snd_pcm_status_user_compat64(struct snd_pcm_substream *substream,
+					struct compat_snd_pcm_status64 __user *src,
+					bool ext)
 {
-	struct snd_pcm_status status;
+	struct snd_pcm_status64 status;
+	struct compat_snd_pcm_status64 compat_status64;
 	int err;
 
 	memset(&status, 0, sizeof(status));
+	memset(&compat_status64, 0, sizeof(compat_status64));
 	/*
 	 * with extension, parameters are read/write,
 	 * get audio_tstamp_data from user,
@@ -286,31 +305,116 @@ static int snd_pcm_status_user_x32(struct snd_pcm_substream *substream,
 	if (ext && get_user(status.audio_tstamp_data,
 				(u32 __user *)(&src->audio_tstamp_data)))
 		return -EFAULT;
-	err = snd_pcm_status(substream, &status);
+	err = snd_pcm_status64(substream, &status);
 	if (err < 0)
 		return err;
 
 	if (clear_user(src, sizeof(*src)))
 		return -EFAULT;
-	if (put_user(status.state, &src->state) ||
-	    put_timespec(&status.trigger_tstamp, &src->trigger_tstamp) ||
-	    put_timespec(&status.tstamp, &src->tstamp) ||
-	    put_user(status.appl_ptr, &src->appl_ptr) ||
-	    put_user(status.hw_ptr, &src->hw_ptr) ||
-	    put_user(status.delay, &src->delay) ||
-	    put_user(status.avail, &src->avail) ||
-	    put_user(status.avail_max, &src->avail_max) ||
-	    put_user(status.overrange, &src->overrange) ||
-	    put_user(status.suspended_state, &src->suspended_state) ||
-	    put_user(status.audio_tstamp_data, &src->audio_tstamp_data) ||
-	    put_timespec(&status.audio_tstamp, &src->audio_tstamp) ||
-	    put_timespec(&status.driver_tstamp, &src->driver_tstamp) ||
-	    put_user(status.audio_tstamp_accuracy, &src->audio_tstamp_accuracy))
+
+	compat_status64 = (struct compat_snd_pcm_status64) {
+		.state = status.state,
+		.trigger_tstamp_sec = status.trigger_tstamp_sec,
+		.trigger_tstamp_nsec = status.trigger_tstamp_nsec,
+		.tstamp_sec = status.tstamp_sec,
+		.tstamp_nsec = status.tstamp_nsec,
+		.appl_ptr = status.appl_ptr,
+		.hw_ptr = status.hw_ptr,
+		.delay = status.delay,
+		.avail = status.avail,
+		.avail_max = status.avail_max,
+		.overrange = status.overrange,
+		.suspended_state = status.suspended_state,
+		.audio_tstamp_data = status.audio_tstamp_data,
+		.audio_tstamp_sec = status.audio_tstamp_sec,
+		.audio_tstamp_nsec = status.audio_tstamp_nsec,
+		.driver_tstamp_sec = status.audio_tstamp_sec,
+		.driver_tstamp_nsec = status.audio_tstamp_nsec,
+		.audio_tstamp_accuracy = status.audio_tstamp_accuracy,
+	};
+
+	if (copy_to_user(src, &compat_status64, sizeof(compat_status64)))
 		return -EFAULT;
 
 	return err;
 }
-#endif /* CONFIG_X86_X32 */
+
+#ifdef IA32_EMULATION
+struct compat_snd_pcm_status64_x86_32 {
+	s32 state;
+	s64 trigger_tstamp_sec;
+	s64 trigger_tstamp_nsec;
+	s64 tstamp_sec;
+	s64 tstamp_nsec;
+	u32 appl_ptr;
+	u32 hw_ptr;
+	s32 delay;
+	u32 avail;
+	u32 avail_max;
+	u32 overrange;
+	s32 suspended_state;
+	u32 audio_tstamp_data;
+	s64 audio_tstamp_sec;
+	s64 audio_tstamp_nsec;
+	s64 driver_tstamp_sec;
+	s64 driver_tstamp_nsec;
+	u32 audio_tstamp_accuracy;
+	unsigned char reserved[52-4*sizeof(s64)];
+} __packed;
+
+static int
+snd_pcm_status_user_compat64_x86_32(struct snd_pcm_substream *substream,
+				    struct compat_snd_pcm_status64_x86_32 __user *src,
+				    bool ext)
+{
+	struct snd_pcm_status64 status;
+	struct compat_snd_pcm_status64_x86_32 status_x86_32;
+	int err;
+
+	memset(&status, 0, sizeof(status));
+	memset(&status_x86_32, 0, sizeof(status_x86_32));
+	/*
+	 * with extension, parameters are read/write,
+	 * get audio_tstamp_data from user,
+	 * ignore rest of status structure
+	 */
+	if (ext && get_user(status.audio_tstamp_data,
+				(u32 __user *)(&src->audio_tstamp_data)))
+		return -EFAULT;
+	err = snd_pcm_status64(substream, &status);
+	if (err < 0)
+		return err;
+
+	if (clear_user(src, sizeof(*src)))
+		return -EFAULT;
+
+	status_x86_32 = (struct compat_snd_pcm_status64_x86_32) {
+		.state = status.state,
+		.trigger_tstamp_sec = status.trigger_tstamp_sec,
+		.trigger_tstamp_nsec = status.trigger_tstamp_nsec,
+		.tstamp_sec = status.tstamp_sec,
+		.tstamp_nsec = status.tstamp_nsec,
+		.appl_ptr = status.appl_ptr,
+		.hw_ptr = status.hw_ptr,
+		.delay = status.delay,
+		.avail = status.avail,
+		.avail_max = status.avail_max,
+		.overrange = status.overrange,
+		.suspended_state = status.suspended_state,
+		.audio_tstamp_data = status.audio_tstamp_data,
+		.audio_tstamp_sec = status.audio_tstamp_sec,
+		.audio_tstamp_nsec = status.audio_tstamp_nsec,
+		.driver_tstamp_sec = status.audio_tstamp_sec,
+		.driver_tstamp_nsec = status.audio_tstamp_nsec,
+		.audio_tstamp_accuracy = status.audio_tstamp_accuracy,
+	};
+
+	if (copy_to_user(src, &status_x86_32, sizeof(status_x86_32)))
+		return -EFAULT;
+
+	return err;
+}
+#endif
 
 /* both for HW_PARAMS and HW_REFINE */
 static int snd_pcm_ioctl_hw_params_compat(struct snd_pcm_substream *substream,
@@ -633,8 +737,8 @@ enum {
 	SNDRV_PCM_IOCTL_HW_REFINE32 = _IOWR('A', 0x10, struct snd_pcm_hw_params32),
 	SNDRV_PCM_IOCTL_HW_PARAMS32 = _IOWR('A', 0x11, struct snd_pcm_hw_params32),
 	SNDRV_PCM_IOCTL_SW_PARAMS32 = _IOWR('A', 0x13, struct snd_pcm_sw_params32),
-	SNDRV_PCM_IOCTL_STATUS32 = _IOR('A', 0x20, struct snd_pcm_status32),
-	SNDRV_PCM_IOCTL_STATUS_EXT32 = _IOWR('A', 0x24, struct snd_pcm_status32),
+	SNDRV_PCM_IOCTL_STATUS_COMPAT32 = _IOR('A', 0x20, struct compat_snd_pcm_status32),
+	SNDRV_PCM_IOCTL_STATUS_EXT_COMPAT32 = _IOWR('A', 0x24, struct compat_snd_pcm_status32),
 	SNDRV_PCM_IOCTL_DELAY32 = _IOR('A', 0x21, s32),
 	SNDRV_PCM_IOCTL_CHANNEL_INFO32 = _IOR('A', 0x32, struct snd_pcm_channel_info32),
 	SNDRV_PCM_IOCTL_REWIND32 = _IOW('A', 0x46, u32),
@@ -644,10 +748,14 @@ enum {
 	SNDRV_PCM_IOCTL_WRITEN_FRAMES32 = _IOW('A', 0x52, struct snd_xfern32),
 	SNDRV_PCM_IOCTL_READN_FRAMES32 = _IOR('A', 0x53, struct snd_xfern32),
 	SNDRV_PCM_IOCTL_SYNC_PTR32 = _IOWR('A', 0x23, struct snd_pcm_sync_ptr32),
+	SNDRV_PCM_IOCTL_STATUS_COMPAT64 = _IOR('A', 0x20, struct compat_snd_pcm_status64),
+	SNDRV_PCM_IOCTL_STATUS_EXT_COMPAT64 = _IOWR('A', 0x24, struct compat_snd_pcm_status64),
+#ifdef IA32_EMULATION
+	SNDRV_PCM_IOCTL_STATUS_COMPAT64_X86_32 = _IOR('A', 0x20, struct compat_snd_pcm_status64_x86_32),
+	SNDRV_PCM_IOCTL_STATUS_EXT_COMPAT64_X86_32 = _IOWR('A', 0x24, struct compat_snd_pcm_status64_x86_32),
+#endif
 #ifdef CONFIG_X86_X32
 	SNDRV_PCM_IOCTL_CHANNEL_INFO_X32 = _IOR('A', 0x32, struct snd_pcm_channel_info),
-	SNDRV_PCM_IOCTL_STATUS_X32 = _IOR('A', 0x20, struct snd_pcm_status_x32),
-	SNDRV_PCM_IOCTL_STATUS_EXT_X32 = _IOWR('A', 0x24, struct snd_pcm_status_x32),
 	SNDRV_PCM_IOCTL_SYNC_PTR_X32 = _IOWR('A', 0x23, struct snd_pcm_sync_ptr_x32),
 #endif /* CONFIG_X86_X32 */
 };
@@ -697,9 +805,9 @@ static long snd_pcm_ioctl_compat(struct file *file, unsigned int cmd, unsigned l
 		return snd_pcm_ioctl_hw_params_compat(substream, 0, argp);
 	case SNDRV_PCM_IOCTL_SW_PARAMS32:
 		return snd_pcm_ioctl_sw_params_compat(substream, argp);
-	case SNDRV_PCM_IOCTL_STATUS32:
+	case SNDRV_PCM_IOCTL_STATUS_COMPAT32:
 		return snd_pcm_status_user_compat(substream, argp, false);
-	case SNDRV_PCM_IOCTL_STATUS_EXT32:
+	case SNDRV_PCM_IOCTL_STATUS_EXT_COMPAT32:
 		return snd_pcm_status_user_compat(substream, argp, true);
 	case SNDRV_PCM_IOCTL_SYNC_PTR32:
 		return snd_pcm_ioctl_sync_ptr_compat(substream, argp);
@@ -719,11 +827,17 @@ static long snd_pcm_ioctl_compat(struct file *file, unsigned int cmd, unsigned l
 		return snd_pcm_ioctl_rewind_compat(substream, argp);
 	case SNDRV_PCM_IOCTL_FORWARD32:
 		return snd_pcm_ioctl_forward_compat(substream, argp);
+	case SNDRV_PCM_IOCTL_STATUS_COMPAT64:
+		return snd_pcm_status_user_compat64(substream, argp, false);
+	case SNDRV_PCM_IOCTL_STATUS_EXT_COMPAT64:
+		return snd_pcm_status_user_compat64(substream, argp, true);
+#ifdef IA32_EMULATION
+	case SNDRV_PCM_IOCTL_STATUS_COMPAT64_X86_32:
+		return snd_pcm_status_user_compat64_x86_32(substream, argp, false);
+	case SNDRV_PCM_IOCTL_STATUS_EXT_COMPAT64_X86_32:
+		return snd_pcm_status_user_compat64_x86_32(substream, argp, true);
+#endif
 #ifdef CONFIG_X86_X32
-	case SNDRV_PCM_IOCTL_STATUS_X32:
-		return snd_pcm_status_user_x32(substream, argp, false);
-	case SNDRV_PCM_IOCTL_STATUS_EXT_X32:
-		return snd_pcm_status_user_x32(substream, argp, true);
 	case SNDRV_PCM_IOCTL_SYNC_PTR_X32:
 		return snd_pcm_ioctl_sync_ptr_x32(substream, argp);
 	case SNDRV_PCM_IOCTL_CHANNEL_INFO_X32:
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 60bc303..7a70848 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -854,8 +854,8 @@ static int snd_pcm_sw_params_user(struct snd_pcm_substream *substream,
 	return err;
 }
 
-int snd_pcm_status(struct snd_pcm_substream *substream,
-		   struct snd_pcm_status *status)
+int snd_pcm_status64(struct snd_pcm_substream *substream,
+		     struct snd_pcm_status64 *status)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
 
@@ -881,14 +881,22 @@ int snd_pcm_status(struct snd_pcm_substream *substream,
 	status->suspended_state = runtime->status->suspended_state;
 	if (status->state == SNDRV_PCM_STATE_OPEN)
 		goto _end;
-	status->trigger_tstamp = timespec64_to_timespec(runtime->trigger_tstamp);
+	status->trigger_tstamp_sec = runtime->trigger_tstamp.tv_sec;
+	status->trigger_tstamp_nsec = runtime->trigger_tstamp.tv_nsec;
 	if (snd_pcm_running(substream)) {
 		snd_pcm_update_hw_ptr(substream);
 		if (runtime->tstamp_mode == SNDRV_PCM_TSTAMP_ENABLE) {
-			status->tstamp = runtime->status->tstamp;
-			status->driver_tstamp = timespec64_to_timespec(runtime->driver_tstamp);
-			status->audio_tstamp =
-				runtime->status->audio_tstamp;
+			status->tstamp_sec = runtime->status->tstamp.tv_sec;
+			status->tstamp_nsec =
+				runtime->status->tstamp.tv_nsec;
+			status->driver_tstamp_sec =
+				runtime->driver_tstamp.tv_sec;
+			status->driver_tstamp_nsec =
+				runtime->driver_tstamp.tv_nsec;
+			status->audio_tstamp_sec =
+				runtime->status->audio_tstamp.tv_sec;
+			status->audio_tstamp_nsec =
+				runtime->status->audio_tstamp.tv_nsec;
 			if (runtime->audio_tstamp_report.valid == 1)
 				/* backwards compatibility, no report provided in COMPAT mode */
 				snd_pcm_pack_audio_tstamp_report(&status->audio_tstamp_data,
@@ -903,7 +911,8 @@ int snd_pcm_status(struct snd_pcm_substream *substream,
 			struct timespec64 tstamp;
 
 			snd_pcm_gettime(runtime, &tstamp);
-			status->tstamp = timespec64_to_timespec(tstamp);
+			status->tstamp_sec = tstamp.tv_sec;
+			status->tstamp_nsec = tstamp.tv_nsec;
 		}
 	}
  _tstamp_end:
@@ -933,11 +942,11 @@ int snd_pcm_status(struct snd_pcm_substream *substream,
 	return 0;
 }
 
-static int snd_pcm_status_user(struct snd_pcm_substream *substream,
-			       struct snd_pcm_status __user * _status,
-			       bool ext)
+static int snd_pcm_status_user64(struct snd_pcm_substream *substream,
+				 struct snd_pcm_status64 __user * _status,
+				 bool ext)
 {
-	struct snd_pcm_status status;
+	struct snd_pcm_status64 status;
 	int res;
 
 	memset(&status, 0, sizeof(status));
@@ -949,7 +958,7 @@ static int snd_pcm_status_user(struct snd_pcm_substream *substream,
 	if (ext && get_user(status.audio_tstamp_data,
 				(u32 __user *)(&_status->audio_tstamp_data)))
 		return -EFAULT;
-	res = snd_pcm_status(substream, &status);
+	res = snd_pcm_status64(substream, &status);
 	if (res < 0)
 		return res;
 	if (copy_to_user(_status, &status, sizeof(status)))
@@ -957,6 +966,57 @@ static int snd_pcm_status_user(struct snd_pcm_substream *substream,
 	return 0;
 }
 
+#if __BITS_PER_LONG == 32
+static int snd_pcm_status_user32(struct snd_pcm_substream *substream,
+				 struct snd_pcm_status32 __user * _status,
+				 bool ext)
+{
+	struct snd_pcm_status64 status64;
+	struct snd_pcm_status32 status32;
+	int res;
+
+	memset(&status64, 0, sizeof(status64));
+	memset(&status32, 0, sizeof(status32));
+	/*
+	 * with extension, parameters are read/write,
+	 * get audio_tstamp_data from user,
+	 * ignore rest of status structure
+	 */
+	if (ext && get_user(status64.audio_tstamp_data,
+			    (u32 __user *)(&_status->audio_tstamp_data)))
+		return -EFAULT;
+	res = snd_pcm_status64(substream, &status64);
+	if (res < 0)
+		return res;
+
+	status32 = (struct snd_pcm_status32) {
+		.state = status64.state,
+		.trigger_tstamp_sec = status64.trigger_tstamp_sec,
+		.trigger_tstamp_nsec = status64.trigger_tstamp_nsec,
+		.tstamp_sec = status64.tstamp_sec,
+		.tstamp_nsec = status64.tstamp_nsec,
+		.appl_ptr = status64.appl_ptr,
+		.hw_ptr = status64.hw_ptr,
+		.delay = status64.delay,
+		.avail = status64.avail,
+		.avail_max = status64.avail_max,
+		.overrange = status64.overrange,
+		.suspended_state = status64.suspended_state,
+		.audio_tstamp_data = status64.audio_tstamp_data,
+		.audio_tstamp_sec = status64.audio_tstamp_sec,
+		.audio_tstamp_nsec = status64.audio_tstamp_nsec,
+		.driver_tstamp_sec = status64.audio_tstamp_sec,
+		.driver_tstamp_nsec = status64.audio_tstamp_nsec,
+		.audio_tstamp_accuracy = status64.audio_tstamp_accuracy,
+	};
+
+	if (copy_to_user(_status, &status32, sizeof(status32)))
+		return -EFAULT;
+
+	return 0;
+}
+#endif
+
 static int snd_pcm_channel_info(struct snd_pcm_substream *substream,
 				struct snd_pcm_channel_info * info)
 {
@@ -2888,10 +2948,16 @@ static int snd_pcm_common_ioctl(struct file *file,
 		return snd_pcm_hw_free(substream);
 	case SNDRV_PCM_IOCTL_SW_PARAMS:
 		return snd_pcm_sw_params_user(substream, arg);
-	case SNDRV_PCM_IOCTL_STATUS:
-		return snd_pcm_status_user(substream, arg, false);
-	case SNDRV_PCM_IOCTL_STATUS_EXT:
-		return snd_pcm_status_user(substream, arg, true);
+#if __BITS_PER_LONG == 32
+	case SNDRV_PCM_IOCTL_STATUS32:
+		return snd_pcm_status_user32(substream, arg, false);
+	case SNDRV_PCM_IOCTL_STATUS_EXT32:
+		return snd_pcm_status_user32(substream, arg, true);
+#endif
+	case SNDRV_PCM_IOCTL_STATUS64:
+		return snd_pcm_status_user64(substream, arg, false);
+	case SNDRV_PCM_IOCTL_STATUS_EXT64:
+		return snd_pcm_status_user64(substream, arg, true);
 	case SNDRV_PCM_IOCTL_CHANNEL_INFO:
 		return snd_pcm_channel_info_user(substream, arg);
 	case SNDRV_PCM_IOCTL_PREPARE:
-- 
1.7.9.5

Re: [alsa-devel] [RFC PATCH v2 2/7] sound: core: Avoid using timespec for struct snd_pcm_status

[Takashi Iwai]

On Thu, 02 Nov 2017 12:06:52 +0100,
Baolin Wang wrote:
> 
> The struct snd_pcm_status will use 'timespec' type variables to record
> timestamp, which is not year 2038 safe on 32bits system.
> 
> Userspace will use SNDRV_PCM_IOCTL_STATUS and SNDRV_PCM_IOCTL_STATUS_EXT
> as commands to issue ioctl() to fill the 'snd_pcm_status' structure in
> userspace. The command number is always defined through _IOR/_IOW/IORW,
> so when userspace changes the definition of 'struct timespec' to use
> 64-bit types, the command number also changes.
> 
> Thus in the kernel, we now need to define two versions of each such ioctl
> and corresponding ioctl commands to handle 32bit time_t and 64bit time_t
> in native mode:
> struct snd_pcm_status32 {
> 	......
> 	struct { s32 tv_sec; s32 tv_nsec; } trigger_tstamp;
> 	struct { s32 tv_sec; s32 tv_nsec; } tstamp;
> 	......
> }
> 
> struct snd_pcm_status64 {
> 	......
> 	struct { s64 tv_sec; s64 tv_nsec; } trigger_tstamp;
> 	struct { s64 tv_sec; s64 tv_nsec; } tstamp;
> 	......
> }
> 
> Moreover in compat file, we renamed or introduced new structures to handle
> 32bit/64bit time_t in compatible mode. 'struct compat_snd_pcm_status32' and
> snd_pcm_status_user_compat() are used to handle 32bit time_t in compat mode.
> 'struct compat_snd_pcm_status64' and snd_pcm_status_user_compat64() are used
> to handle 64bit time_t with 64bit alignment. 'struct compat_snd_pcm_status64_x86_32'
> and snd_pcm_status_user_compat64_x86_32() are used to handle 64bit time_t with
> 32bit alignment.

Hmm...  I don't get why you need to redefine these in
include/sound/pcm.h and define even IOCTLs there.  These are the
things for ABI, no?  If yes, we don't need to have it globally in the
public header but define/convert them locally.  And, renaming
snd_pcm_status64 allover the places for internal doesn't look good.


thanks,

Takashi

> 
> Finally we can replace SNDRV_PCM_IOCTL_STATUS and SNDRV_PCM_IOCTL_STATUS_EXT
> with new commands and introduce new functions to fill new 'struct snd_pcm_status64'
> instead of using unsafe 'struct snd_pcm_status'. Then in future, the new
> commands can be matched when userspace changes 'timespec' to 64bit type
> to make a size change of 'struct snd_pcm_status'. When glibc changes time_t
> to 64-bit, any recompiled program will issue ioctl commands that the kernel
> does not understand without this patch.
> 
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---
>  include/sound/pcm.h     |   57 +++++++++++-
>  sound/core/pcm.c        |   12 +--
>  sound/core/pcm_compat.c |  236 +++++++++++++++++++++++++++++++++++------------
>  sound/core/pcm_native.c |  100 ++++++++++++++++----
>  4 files changed, 319 insertions(+), 86 deletions(-)
> 
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index cd1ecd6..7524b54 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -58,6 +58,7 @@ struct snd_pcm_hardware {
>  	size_t fifo_size;		/* fifo size in bytes */
>  };
>  
> +struct snd_pcm_status64;
>  struct snd_pcm_substream;
>  
>  struct snd_pcm_audio_tstamp_config; /* definitions further down */
> @@ -565,8 +566,8 @@ struct snd_pcm_notify {
>  int snd_pcm_info(struct snd_pcm_substream *substream, struct snd_pcm_info *info);
>  int snd_pcm_info_user(struct snd_pcm_substream *substream,
>  		      struct snd_pcm_info __user *info);
> -int snd_pcm_status(struct snd_pcm_substream *substream,
> -		   struct snd_pcm_status *status);
> +int snd_pcm_status64(struct snd_pcm_substream *substream,
> +		     struct snd_pcm_status64 *status);
>  int snd_pcm_start(struct snd_pcm_substream *substream);
>  int snd_pcm_stop(struct snd_pcm_substream *substream, snd_pcm_state_t status);
>  int snd_pcm_drain_done(struct snd_pcm_substream *substream);
> @@ -1440,4 +1441,56 @@ static inline u64 pcm_format_to_bits(snd_pcm_format_t pcm_format)
>  #define pcm_dbg(pcm, fmt, args...) \
>  	dev_dbg((pcm)->card->dev, fmt, ##args)
>  
> +struct snd_pcm_status64 {
> +	snd_pcm_state_t state;		/* stream state */
> +	s64 trigger_tstamp_sec;		/* time when stream was started/stopped/paused */
> +	s64 trigger_tstamp_nsec;
> +	s64 tstamp_sec;			/* reference timestamp */
> +	s64 tstamp_nsec;
> +	snd_pcm_uframes_t appl_ptr;	/* appl ptr */
> +	snd_pcm_uframes_t hw_ptr;	/* hw ptr */
> +	snd_pcm_sframes_t delay;	/* current delay in frames */
> +	snd_pcm_uframes_t avail;	/* number of frames available */
> +	snd_pcm_uframes_t avail_max;	/* max frames available on hw since last status */
> +	snd_pcm_uframes_t overrange;	/* count of ADC (capture) overrange detections from last status */
> +	snd_pcm_state_t suspended_state; /* suspended stream state */
> +	__u32 audio_tstamp_data;	 /* needed for 64-bit alignment, used for configs/report to/from userspace */
> +	s64 audio_tstamp_sec;		/* sample counter, wall clock, PHC or on-demand sync'ed */
> +	s64 audio_tstamp_nsec;
> +	s64 driver_tstamp_sec;		/* useful in case reference system tstamp is reported with delay */
> +	s64 driver_tstamp_nsec;
> +	__u32 audio_tstamp_accuracy;	/* in ns units, only valid if indicated in audio_tstamp_data */
> +	unsigned char reserved[52-4*sizeof(s64)]; /* must be filled with zero */
> +};
> +
> +#define SNDRV_PCM_IOCTL_STATUS64	_IOR('A', 0x20, struct snd_pcm_status64)
> +#define SNDRV_PCM_IOCTL_STATUS_EXT64	_IOWR('A', 0x24, struct snd_pcm_status64)
> +
> +#if __BITS_PER_LONG == 32
> +struct snd_pcm_status32 {
> +	snd_pcm_state_t state;		/* stream state */
> +	s32 trigger_tstamp_sec;		/* time when stream was started/stopped/paused */
> +	s32 trigger_tstamp_nsec;
> +	s32 tstamp_sec;			/* reference timestamp */
> +	s32 tstamp_nsec;
> +	snd_pcm_uframes_t appl_ptr;	/* appl ptr */
> +	snd_pcm_uframes_t hw_ptr;	/* hw ptr */
> +	snd_pcm_sframes_t delay;	/* current delay in frames */
> +	snd_pcm_uframes_t avail;	/* number of frames available */
> +	snd_pcm_uframes_t avail_max;	/* max frames available on hw since last status */
> +	snd_pcm_uframes_t overrange;	/* count of ADC (capture) overrange detections from last status */
> +	snd_pcm_state_t suspended_state; /* suspended stream state */
> +	__u32 audio_tstamp_data;	 /* needed for 64-bit alignment, used for configs/report to/from userspace */
> +	s32 audio_tstamp_sec;		/* sample counter, wall clock, PHC or on-demand sync'ed */
> +	s32 audio_tstamp_nsec;
> +	s32 driver_tstamp_sec;		/* useful in case reference system tstamp is reported with delay */
> +	s32 driver_tstamp_nsec;
> +	__u32 audio_tstamp_accuracy;	/* in ns units, only valid if indicated in audio_tstamp_data */
> +	unsigned char reserved[52-4*sizeof(s32)]; /* must be filled with zero */
> +};
> +
> +#define SNDRV_PCM_IOCTL_STATUS32	_IOR('A', 0x20, struct snd_pcm_status32)
> +#define SNDRV_PCM_IOCTL_STATUS_EXT32	_IOWR('A', 0x24, struct snd_pcm_status32)
> +#endif
> +
>  #endif /* __SOUND_PCM_H */
> diff --git a/sound/core/pcm.c b/sound/core/pcm.c
> index 7eadb7f..c19e656 100644
> --- a/sound/core/pcm.c
> +++ b/sound/core/pcm.c
> @@ -453,7 +453,7 @@ static void snd_pcm_substream_proc_status_read(struct snd_info_entry *entry,
>  {
>  	struct snd_pcm_substream *substream = entry->private_data;
>  	struct snd_pcm_runtime *runtime;
> -	struct snd_pcm_status status;
> +	struct snd_pcm_status64 status;
>  	int err;
>  
>  	mutex_lock(&substream->pcm->open_mutex);
> @@ -463,17 +463,17 @@ static void snd_pcm_substream_proc_status_read(struct snd_info_entry *entry,
>  		goto unlock;
>  	}
>  	memset(&status, 0, sizeof(status));
> -	err = snd_pcm_status(substream, &status);
> +	err = snd_pcm_status64(substream, &status);
>  	if (err < 0) {
>  		snd_iprintf(buffer, "error %d\n", err);
>  		goto unlock;
>  	}
>  	snd_iprintf(buffer, "state: %s\n", snd_pcm_state_name(status.state));
>  	snd_iprintf(buffer, "owner_pid   : %d\n", pid_vnr(substream->pid));
> -	snd_iprintf(buffer, "trigger_time: %ld.%09ld\n",
> -		status.trigger_tstamp.tv_sec, status.trigger_tstamp.tv_nsec);
> -	snd_iprintf(buffer, "tstamp      : %ld.%09ld\n",
> -		status.tstamp.tv_sec, status.tstamp.tv_nsec);
> +	snd_iprintf(buffer, "trigger_time: %lld.%09lld\n",
> +		status.trigger_tstamp_sec, status.trigger_tstamp_nsec);
> +	snd_iprintf(buffer, "tstamp      : %lld.%09lld\n",
> +		status.tstamp_sec, status.tstamp_nsec);
>  	snd_iprintf(buffer, "delay       : %ld\n", status.delay);
>  	snd_iprintf(buffer, "avail       : %ld\n", status.avail);
>  	snd_iprintf(buffer, "avail_max   : %ld\n", status.avail_max);
> diff --git a/sound/core/pcm_compat.c b/sound/core/pcm_compat.c
> index b719d0b..53b83d4 100644
> --- a/sound/core/pcm_compat.c
> +++ b/sound/core/pcm_compat.c
> @@ -187,10 +187,12 @@ static int snd_pcm_channel_info_user(struct snd_pcm_substream *substream,
>  	snd_pcm_channel_info_user(s, p)
>  #endif /* CONFIG_X86_X32 */
>  
> -struct snd_pcm_status32 {
> +struct compat_snd_pcm_status32 {
>  	s32 state;
> -	struct compat_timespec trigger_tstamp;
> -	struct compat_timespec tstamp;
> +	s32 trigger_tstamp_sec;
> +	s32 trigger_tstamp_nsec;
> +	s32 tstamp_sec;
> +	s32 tstamp_nsec;
>  	u32 appl_ptr;
>  	u32 hw_ptr;
>  	s32 delay;
> @@ -199,21 +201,25 @@ struct snd_pcm_status32 {
>  	u32 overrange;
>  	s32 suspended_state;
>  	u32 audio_tstamp_data;
> -	struct compat_timespec audio_tstamp;
> -	struct compat_timespec driver_tstamp;
> +	s32 audio_tstamp_sec;
> +	s32 audio_tstamp_nsec;
> +	s32 driver_tstamp_sec;
> +	s32 driver_tstamp_nsec;
>  	u32 audio_tstamp_accuracy;
> -	unsigned char reserved[52-2*sizeof(struct compat_timespec)];
> +	unsigned char reserved[52-4*sizeof(s32)];
>  } __attribute__((packed));
>  
>  
>  static int snd_pcm_status_user_compat(struct snd_pcm_substream *substream,
> -				      struct snd_pcm_status32 __user *src,
> +				      struct compat_snd_pcm_status32 __user *src,
>  				      bool ext)
>  {
> -	struct snd_pcm_status status;
> +	struct snd_pcm_status64 status;
> +	struct compat_snd_pcm_status32 compat_status32;
>  	int err;
>  
>  	memset(&status, 0, sizeof(status));
> +	memset(&compat_status32, 0, sizeof(compat_status32));
>  	/*
>  	 * with extension, parameters are read/write,
>  	 * get audio_tstamp_data from user,
> @@ -222,38 +228,47 @@ static int snd_pcm_status_user_compat(struct snd_pcm_substream *substream,
>  	if (ext && get_user(status.audio_tstamp_data,
>  				(u32 __user *)(&src->audio_tstamp_data)))
>  		return -EFAULT;
> -	err = snd_pcm_status(substream, &status);
> +	err = snd_pcm_status64(substream, &status);
>  	if (err < 0)
>  		return err;
>  
>  	if (clear_user(src, sizeof(*src)))
>  		return -EFAULT;
> -	if (put_user(status.state, &src->state) ||
> -	    compat_put_timespec(&status.trigger_tstamp, &src->trigger_tstamp) ||
> -	    compat_put_timespec(&status.tstamp, &src->tstamp) ||
> -	    put_user(status.appl_ptr, &src->appl_ptr) ||
> -	    put_user(status.hw_ptr, &src->hw_ptr) ||
> -	    put_user(status.delay, &src->delay) ||
> -	    put_user(status.avail, &src->avail) ||
> -	    put_user(status.avail_max, &src->avail_max) ||
> -	    put_user(status.overrange, &src->overrange) ||
> -	    put_user(status.suspended_state, &src->suspended_state) ||
> -	    put_user(status.audio_tstamp_data, &src->audio_tstamp_data) ||
> -	    compat_put_timespec(&status.audio_tstamp, &src->audio_tstamp) ||
> -	    compat_put_timespec(&status.driver_tstamp, &src->driver_tstamp) ||
> -	    put_user(status.audio_tstamp_accuracy, &src->audio_tstamp_accuracy))
> +
> +	compat_status32 = (struct compat_snd_pcm_status32) {
> +		.state = status.state,
> +		.trigger_tstamp_sec = status.trigger_tstamp_sec,
> +		.trigger_tstamp_nsec = status.trigger_tstamp_nsec,
> +		.tstamp_sec = status.tstamp_sec,
> +		.tstamp_nsec = status.tstamp_nsec,
> +		.appl_ptr = status.appl_ptr,
> +		.hw_ptr = status.hw_ptr,
> +		.delay = status.delay,
> +		.avail = status.avail,
> +		.avail_max = status.avail_max,
> +		.overrange = status.overrange,
> +		.suspended_state = status.suspended_state,
> +		.audio_tstamp_data = status.audio_tstamp_data,
> +		.audio_tstamp_sec = status.audio_tstamp_sec,
> +		.audio_tstamp_nsec = status.audio_tstamp_nsec,
> +		.driver_tstamp_sec = status.audio_tstamp_sec,
> +		.driver_tstamp_nsec = status.audio_tstamp_nsec,
> +		.audio_tstamp_accuracy = status.audio_tstamp_accuracy,
> +	};
> +
> +	if (copy_to_user(src, &compat_status32, sizeof(compat_status32)))
>  		return -EFAULT;
>  
>  	return err;
>  }
>  
> -#ifdef CONFIG_X86_X32
> -/* X32 ABI has 64bit timespec and 64bit alignment */
> -struct snd_pcm_status_x32 {
> +struct compat_snd_pcm_status64 {
>  	s32 state;
>  	u32 rsvd; /* alignment */
> -	struct timespec trigger_tstamp;
> -	struct timespec tstamp;
> +	s64 trigger_tstamp_sec;
> +	s64 trigger_tstamp_nsec;
> +	s64 tstamp_sec;
> +	s64 tstamp_nsec;
>  	u32 appl_ptr;
>  	u32 hw_ptr;
>  	s32 delay;
> @@ -262,22 +277,26 @@ struct snd_pcm_status_x32 {
>  	u32 overrange;
>  	s32 suspended_state;
>  	u32 audio_tstamp_data;
> -	struct timespec audio_tstamp;
> -	struct timespec driver_tstamp;
> +	s64 audio_tstamp_sec;
> +	s64 audio_tstamp_nsec;
> +	s64 driver_tstamp_sec;
> +	s64 driver_tstamp_nsec;
>  	u32 audio_tstamp_accuracy;
> -	unsigned char reserved[52-2*sizeof(struct timespec)];
> +	unsigned char reserved[52-4*sizeof(s64)];
>  } __packed;
>  
>  #define put_timespec(src, dst) copy_to_user(dst, src, sizeof(*dst))
>  
> -static int snd_pcm_status_user_x32(struct snd_pcm_substream *substream,
> -				   struct snd_pcm_status_x32 __user *src,
> -				   bool ext)
> +static int snd_pcm_status_user_compat64(struct snd_pcm_substream *substream,
> +					struct compat_snd_pcm_status64 __user *src,
> +					bool ext)
>  {
> -	struct snd_pcm_status status;
> +	struct snd_pcm_status64 status;
> +	struct compat_snd_pcm_status64 compat_status64;
>  	int err;
>  
>  	memset(&status, 0, sizeof(status));
> +	memset(&compat_status64, 0, sizeof(compat_status64));
>  	/*
>  	 * with extension, parameters are read/write,
>  	 * get audio_tstamp_data from user,
> @@ -286,31 +305,116 @@ static int snd_pcm_status_user_x32(struct snd_pcm_substream *substream,
>  	if (ext && get_user(status.audio_tstamp_data,
>  				(u32 __user *)(&src->audio_tstamp_data)))
>  		return -EFAULT;
> -	err = snd_pcm_status(substream, &status);
> +	err = snd_pcm_status64(substream, &status);
>  	if (err < 0)
>  		return err;
>  
>  	if (clear_user(src, sizeof(*src)))
>  		return -EFAULT;
> -	if (put_user(status.state, &src->state) ||
> -	    put_timespec(&status.trigger_tstamp, &src->trigger_tstamp) ||
> -	    put_timespec(&status.tstamp, &src->tstamp) ||
> -	    put_user(status.appl_ptr, &src->appl_ptr) ||
> -	    put_user(status.hw_ptr, &src->hw_ptr) ||
> -	    put_user(status.delay, &src->delay) ||
> -	    put_user(status.avail, &src->avail) ||
> -	    put_user(status.avail_max, &src->avail_max) ||
> -	    put_user(status.overrange, &src->overrange) ||
> -	    put_user(status.suspended_state, &src->suspended_state) ||
> -	    put_user(status.audio_tstamp_data, &src->audio_tstamp_data) ||
> -	    put_timespec(&status.audio_tstamp, &src->audio_tstamp) ||
> -	    put_timespec(&status.driver_tstamp, &src->driver_tstamp) ||
> -	    put_user(status.audio_tstamp_accuracy, &src->audio_tstamp_accuracy))
> +
> +	compat_status64 = (struct compat_snd_pcm_status64) {
> +		.state = status.state,
> +		.trigger_tstamp_sec = status.trigger_tstamp_sec,
> +		.trigger_tstamp_nsec = status.trigger_tstamp_nsec,
> +		.tstamp_sec = status.tstamp_sec,
> +		.tstamp_nsec = status.tstamp_nsec,
> +		.appl_ptr = status.appl_ptr,
> +		.hw_ptr = status.hw_ptr,
> +		.delay = status.delay,
> +		.avail = status.avail,
> +		.avail_max = status.avail_max,
> +		.overrange = status.overrange,
> +		.suspended_state = status.suspended_state,
> +		.audio_tstamp_data = status.audio_tstamp_data,
> +		.audio_tstamp_sec = status.audio_tstamp_sec,
> +		.audio_tstamp_nsec = status.audio_tstamp_nsec,
> +		.driver_tstamp_sec = status.audio_tstamp_sec,
> +		.driver_tstamp_nsec = status.audio_tstamp_nsec,
> +		.audio_tstamp_accuracy = status.audio_tstamp_accuracy,
> +	};
> +
> +	if (copy_to_user(src, &compat_status64, sizeof(compat_status64)))
>  		return -EFAULT;
>  
>  	return err;
>  }
> -#endif /* CONFIG_X86_X32 */
> +
> +#ifdef IA32_EMULATION
> +struct compat_snd_pcm_status64_x86_32 {
> +	s32 state;
> +	s64 trigger_tstamp_sec;
> +	s64 trigger_tstamp_nsec;
> +	s64 tstamp_sec;
> +	s64 tstamp_nsec;
> +	u32 appl_ptr;
> +	u32 hw_ptr;
> +	s32 delay;
> +	u32 avail;
> +	u32 avail_max;
> +	u32 overrange;
> +	s32 suspended_state;
> +	u32 audio_tstamp_data;
> +	s64 audio_tstamp_sec;
> +	s64 audio_tstamp_nsec;
> +	s64 driver_tstamp_sec;
> +	s64 driver_tstamp_nsec;
> +	u32 audio_tstamp_accuracy;
> +	unsigned char reserved[52-4*sizeof(s64)];
> +} __packed;
> +
> +static int
> +snd_pcm_status_user_compat64_x86_32(struct snd_pcm_substream *substream,
> +				    struct compat_snd_pcm_status64_x86_32 __user *src,
> +				    bool ext)
> +{
> +	struct snd_pcm_status64 status;
> +	struct compat_snd_pcm_status64_x86_32 status_x86_32;
> +	int err;
> +
> +	memset(&status, 0, sizeof(status));
> +	memset(&status_x86_32, 0, sizeof(status_x86_32));
> +	/*
> +	 * with extension, parameters are read/write,
> +	 * get audio_tstamp_data from user,
> +	 * ignore rest of status structure
> +	 */
> +	if (ext && get_user(status.audio_tstamp_data,
> +				(u32 __user *)(&src->audio_tstamp_data)))
> +		return -EFAULT;
> +	err = snd_pcm_status64(substream, &status);
> +	if (err < 0)
> +		return err;
> +
> +	if (clear_user(src, sizeof(*src)))
> +		return -EFAULT;
> +
> +	status_x86_32 = (struct compat_snd_pcm_status64_x86_32) {
> +		.state = status.state,
> +		.trigger_tstamp_sec = status.trigger_tstamp_sec,
> +		.trigger_tstamp_nsec = status.trigger_tstamp_nsec,
> +		.tstamp_sec = status.tstamp_sec,
> +		.tstamp_nsec = status.tstamp_nsec,
> +		.appl_ptr = status.appl_ptr,
> +		.hw_ptr = status.hw_ptr,
> +		.delay = status.delay,
> +		.avail = status.avail,
> +		.avail_max = status.avail_max,
> +		.overrange = status.overrange,
> +		.suspended_state = status.suspended_state,
> +		.audio_tstamp_data = status.audio_tstamp_data,
> +		.audio_tstamp_sec = status.audio_tstamp_sec,
> +		.audio_tstamp_nsec = status.audio_tstamp_nsec,
> +		.driver_tstamp_sec = status.audio_tstamp_sec,
> +		.driver_tstamp_nsec = status.audio_tstamp_nsec,
> +		.audio_tstamp_accuracy = status.audio_tstamp_accuracy,
> +	};
> +
> +	if (copy_to_user(src, &status_x86_32, sizeof(status_x86_32)))
> +		return -EFAULT;
> +
> +	return err;
> +}
> +#endif
>  
>  /* both for HW_PARAMS and HW_REFINE */
>  static int snd_pcm_ioctl_hw_params_compat(struct snd_pcm_substream *substream,
> @@ -633,8 +737,8 @@ enum {
>  	SNDRV_PCM_IOCTL_HW_REFINE32 = _IOWR('A', 0x10, struct snd_pcm_hw_params32),
>  	SNDRV_PCM_IOCTL_HW_PARAMS32 = _IOWR('A', 0x11, struct snd_pcm_hw_params32),
>  	SNDRV_PCM_IOCTL_SW_PARAMS32 = _IOWR('A', 0x13, struct snd_pcm_sw_params32),
> -	SNDRV_PCM_IOCTL_STATUS32 = _IOR('A', 0x20, struct snd_pcm_status32),
> -	SNDRV_PCM_IOCTL_STATUS_EXT32 = _IOWR('A', 0x24, struct snd_pcm_status32),
> +	SNDRV_PCM_IOCTL_STATUS_COMPAT32 = _IOR('A', 0x20, struct compat_snd_pcm_status32),
> +	SNDRV_PCM_IOCTL_STATUS_EXT_COMPAT32 = _IOWR('A', 0x24, struct compat_snd_pcm_status32),
>  	SNDRV_PCM_IOCTL_DELAY32 = _IOR('A', 0x21, s32),
>  	SNDRV_PCM_IOCTL_CHANNEL_INFO32 = _IOR('A', 0x32, struct snd_pcm_channel_info32),
>  	SNDRV_PCM_IOCTL_REWIND32 = _IOW('A', 0x46, u32),
> @@ -644,10 +748,14 @@ enum {
>  	SNDRV_PCM_IOCTL_WRITEN_FRAMES32 = _IOW('A', 0x52, struct snd_xfern32),
>  	SNDRV_PCM_IOCTL_READN_FRAMES32 = _IOR('A', 0x53, struct snd_xfern32),
>  	SNDRV_PCM_IOCTL_SYNC_PTR32 = _IOWR('A', 0x23, struct snd_pcm_sync_ptr32),
> +	SNDRV_PCM_IOCTL_STATUS_COMPAT64 = _IOR('A', 0x20, struct compat_snd_pcm_status64),
> +	SNDRV_PCM_IOCTL_STATUS_EXT_COMPAT64 = _IOWR('A', 0x24, struct compat_snd_pcm_status64),
> +#ifdef IA32_EMULATION
> +	SNDRV_PCM_IOCTL_STATUS_COMPAT64_X86_32 = _IOR('A', 0x20, struct compat_snd_pcm_status64_x86_32),
> +	SNDRV_PCM_IOCTL_STATUS_EXT_COMPAT64_X86_32 = _IOWR('A', 0x24, struct compat_snd_pcm_status64_x86_32),
> +#endif
>  #ifdef CONFIG_X86_X32
>  	SNDRV_PCM_IOCTL_CHANNEL_INFO_X32 = _IOR('A', 0x32, struct snd_pcm_channel_info),
> -	SNDRV_PCM_IOCTL_STATUS_X32 = _IOR('A', 0x20, struct snd_pcm_status_x32),
> -	SNDRV_PCM_IOCTL_STATUS_EXT_X32 = _IOWR('A', 0x24, struct snd_pcm_status_x32),
>  	SNDRV_PCM_IOCTL_SYNC_PTR_X32 = _IOWR('A', 0x23, struct snd_pcm_sync_ptr_x32),
>  #endif /* CONFIG_X86_X32 */
>  };
> @@ -697,9 +805,9 @@ static long snd_pcm_ioctl_compat(struct file *file, unsigned int cmd, unsigned l
>  		return snd_pcm_ioctl_hw_params_compat(substream, 0, argp);
>  	case SNDRV_PCM_IOCTL_SW_PARAMS32:
>  		return snd_pcm_ioctl_sw_params_compat(substream, argp);
> -	case SNDRV_PCM_IOCTL_STATUS32:
> +	case SNDRV_PCM_IOCTL_STATUS_COMPAT32:
>  		return snd_pcm_status_user_compat(substream, argp, false);
> -	case SNDRV_PCM_IOCTL_STATUS_EXT32:
> +	case SNDRV_PCM_IOCTL_STATUS_EXT_COMPAT32:
>  		return snd_pcm_status_user_compat(substream, argp, true);
>  	case SNDRV_PCM_IOCTL_SYNC_PTR32:
>  		return snd_pcm_ioctl_sync_ptr_compat(substream, argp);
> @@ -719,11 +827,17 @@ static long snd_pcm_ioctl_compat(struct file *file, unsigned int cmd, unsigned l
>  		return snd_pcm_ioctl_rewind_compat(substream, argp);
>  	case SNDRV_PCM_IOCTL_FORWARD32:
>  		return snd_pcm_ioctl_forward_compat(substream, argp);
> +	case SNDRV_PCM_IOCTL_STATUS_COMPAT64:
> +		return snd_pcm_status_user_compat64(substream, argp, false);
> +	case SNDRV_PCM_IOCTL_STATUS_EXT_COMPAT64:
> +		return snd_pcm_status_user_compat64(substream, argp, true);
> +#ifdef IA32_EMULATION
> +	case SNDRV_PCM_IOCTL_STATUS_COMPAT64_X86_32:
> +		return snd_pcm_status_user_compat64_x86_32(substream, argp, false);
> +	case SNDRV_PCM_IOCTL_STATUS_EXT_COMPAT64_X86_32:
> +		return snd_pcm_status_user_compat64_x86_32(substream, argp, true);
> +#endif
>  #ifdef CONFIG_X86_X32
> -	case SNDRV_PCM_IOCTL_STATUS_X32:
> -		return snd_pcm_status_user_x32(substream, argp, false);
> -	case SNDRV_PCM_IOCTL_STATUS_EXT_X32:
> -		return snd_pcm_status_user_x32(substream, argp, true);
>  	case SNDRV_PCM_IOCTL_SYNC_PTR_X32:
>  		return snd_pcm_ioctl_sync_ptr_x32(substream, argp);
>  	case SNDRV_PCM_IOCTL_CHANNEL_INFO_X32:
> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> index 60bc303..7a70848 100644
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -854,8 +854,8 @@ static int snd_pcm_sw_params_user(struct snd_pcm_substream *substream,
>  	return err;
>  }
>  
> -int snd_pcm_status(struct snd_pcm_substream *substream,
> -		   struct snd_pcm_status *status)
> +int snd_pcm_status64(struct snd_pcm_substream *substream,
> +		     struct snd_pcm_status64 *status)
>  {
>  	struct snd_pcm_runtime *runtime = substream->runtime;
>  
> @@ -881,14 +881,22 @@ int snd_pcm_status(struct snd_pcm_substream *substream,
>  	status->suspended_state = runtime->status->suspended_state;
>  	if (status->state == SNDRV_PCM_STATE_OPEN)
>  		goto _end;
> -	status->trigger_tstamp = timespec64_to_timespec(runtime->trigger_tstamp);
> +	status->trigger_tstamp_sec = runtime->trigger_tstamp.tv_sec;
> +	status->trigger_tstamp_nsec = runtime->trigger_tstamp.tv_nsec;
>  	if (snd_pcm_running(substream)) {
>  		snd_pcm_update_hw_ptr(substream);
>  		if (runtime->tstamp_mode == SNDRV_PCM_TSTAMP_ENABLE) {
> -			status->tstamp = runtime->status->tstamp;
> -			status->driver_tstamp = timespec64_to_timespec(runtime->driver_tstamp);
> -			status->audio_tstamp =
> -				runtime->status->audio_tstamp;
> +			status->tstamp_sec = runtime->status->tstamp.tv_sec;
> +			status->tstamp_nsec =
> +				runtime->status->tstamp.tv_nsec;
> +			status->driver_tstamp_sec =
> +				runtime->driver_tstamp.tv_sec;
> +			status->driver_tstamp_nsec =
> +				runtime->driver_tstamp.tv_nsec;
> +			status->audio_tstamp_sec =
> +				runtime->status->audio_tstamp.tv_sec;
> +			status->audio_tstamp_nsec =
> +				runtime->status->audio_tstamp.tv_nsec;
>  			if (runtime->audio_tstamp_report.valid == 1)
>  				/* backwards compatibility, no report provided in COMPAT mode */
>  				snd_pcm_pack_audio_tstamp_report(&status->audio_tstamp_data,
> @@ -903,7 +911,8 @@ int snd_pcm_status(struct snd_pcm_substream *substream,
>  			struct timespec64 tstamp;
>  
>  			snd_pcm_gettime(runtime, &tstamp);
> -			status->tstamp = timespec64_to_timespec(tstamp);
> +			status->tstamp_sec = tstamp.tv_sec;
> +			status->tstamp_nsec = tstamp.tv_nsec;
>  		}
>  	}
>   _tstamp_end:
> @@ -933,11 +942,11 @@ int snd_pcm_status(struct snd_pcm_substream *substream,
>  	return 0;
>  }
>  
> -static int snd_pcm_status_user(struct snd_pcm_substream *substream,
> -			       struct snd_pcm_status __user * _status,
> -			       bool ext)
> +static int snd_pcm_status_user64(struct snd_pcm_substream *substream,
> +				 struct snd_pcm_status64 __user * _status,
> +				 bool ext)
>  {
> -	struct snd_pcm_status status;
> +	struct snd_pcm_status64 status;
>  	int res;
>  
>  	memset(&status, 0, sizeof(status));
> @@ -949,7 +958,7 @@ static int snd_pcm_status_user(struct snd_pcm_substream *substream,
>  	if (ext && get_user(status.audio_tstamp_data,
>  				(u32 __user *)(&_status->audio_tstamp_data)))
>  		return -EFAULT;
> -	res = snd_pcm_status(substream, &status);
> +	res = snd_pcm_status64(substream, &status);
>  	if (res < 0)
>  		return res;
>  	if (copy_to_user(_status, &status, sizeof(status)))
> @@ -957,6 +966,57 @@ static int snd_pcm_status_user(struct snd_pcm_substream *substream,
>  	return 0;
>  }
>  
> +#if __BITS_PER_LONG == 32
> +static int snd_pcm_status_user32(struct snd_pcm_substream *substream,
> +				 struct snd_pcm_status32 __user * _status,
> +				 bool ext)
> +{
> +	struct snd_pcm_status64 status64;
> +	struct snd_pcm_status32 status32;
> +	int res;
> +
> +	memset(&status64, 0, sizeof(status64));
> +	memset(&status32, 0, sizeof(status32));
> +	/*
> +	 * with extension, parameters are read/write,
> +	 * get audio_tstamp_data from user,
> +	 * ignore rest of status structure
> +	 */
> +	if (ext && get_user(status64.audio_tstamp_data,
> +			    (u32 __user *)(&_status->audio_tstamp_data)))
> +		return -EFAULT;
> +	res = snd_pcm_status64(substream, &status64);
> +	if (res < 0)
> +		return res;
> +
> +	status32 = (struct snd_pcm_status32) {
> +		.state = status64.state,
> +		.trigger_tstamp_sec = status64.trigger_tstamp_sec,
> +		.trigger_tstamp_nsec = status64.trigger_tstamp_nsec,
> +		.tstamp_sec = status64.tstamp_sec,
> +		.tstamp_nsec = status64.tstamp_nsec,
> +		.appl_ptr = status64.appl_ptr,
> +		.hw_ptr = status64.hw_ptr,
> +		.delay = status64.delay,
> +		.avail = status64.avail,
> +		.avail_max = status64.avail_max,
> +		.overrange = status64.overrange,
> +		.suspended_state = status64.suspended_state,
> +		.audio_tstamp_data = status64.audio_tstamp_data,
> +		.audio_tstamp_sec = status64.audio_tstamp_sec,
> +		.audio_tstamp_nsec = status64.audio_tstamp_nsec,
> +		.driver_tstamp_sec = status64.audio_tstamp_sec,
> +		.driver_tstamp_nsec = status64.audio_tstamp_nsec,
> +		.audio_tstamp_accuracy = status64.audio_tstamp_accuracy,
> +	};
> +
> +	if (copy_to_user(_status, &status32, sizeof(status32)))
> +		return -EFAULT;
> +
> +	return 0;
> +}
> +#endif
> +
>  static int snd_pcm_channel_info(struct snd_pcm_substream *substream,
>  				struct snd_pcm_channel_info * info)
>  {
> @@ -2888,10 +2948,16 @@ static int snd_pcm_common_ioctl(struct file *file,
>  		return snd_pcm_hw_free(substream);
>  	case SNDRV_PCM_IOCTL_SW_PARAMS:
>  		return snd_pcm_sw_params_user(substream, arg);
> -	case SNDRV_PCM_IOCTL_STATUS:
> -		return snd_pcm_status_user(substream, arg, false);
> -	case SNDRV_PCM_IOCTL_STATUS_EXT:
> -		return snd_pcm_status_user(substream, arg, true);
> +#if __BITS_PER_LONG == 32
> +	case SNDRV_PCM_IOCTL_STATUS32:
> +		return snd_pcm_status_user32(substream, arg, false);
> +	case SNDRV_PCM_IOCTL_STATUS_EXT32:
> +		return snd_pcm_status_user32(substream, arg, true);
> +#endif
> +	case SNDRV_PCM_IOCTL_STATUS64:
> +		return snd_pcm_status_user64(substream, arg, false);
> +	case SNDRV_PCM_IOCTL_STATUS_EXT64:
> +		return snd_pcm_status_user64(substream, arg, true);
>  	case SNDRV_PCM_IOCTL_CHANNEL_INFO:
>  		return snd_pcm_channel_info_user(substream, arg);
>  	case SNDRV_PCM_IOCTL_PREPARE:
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

Re: [alsa-devel] [RFC PATCH v2 2/7] sound: core: Avoid using timespec for struct snd_pcm_status

[Arnd Bergmann]

On Sun, Nov 5, 2017 at 11:23 AM, Takashi Iwai <tiwai@suse.de> wrote:
> On Thu, 02 Nov 2017 12:06:52 +0100,
> Baolin Wang wrote:
>>
>> The struct snd_pcm_status will use 'timespec' type variables to record
>> timestamp, which is not year 2038 safe on 32bits system.
>>
>> Userspace will use SNDRV_PCM_IOCTL_STATUS and SNDRV_PCM_IOCTL_STATUS_EXT
>> as commands to issue ioctl() to fill the 'snd_pcm_status' structure in
>> userspace. The command number is always defined through _IOR/_IOW/IORW,
>> so when userspace changes the definition of 'struct timespec' to use
>> 64-bit types, the command number also changes.
>>
>> Thus in the kernel, we now need to define two versions of each such ioctl
>> and corresponding ioctl commands to handle 32bit time_t and 64bit time_t
>> in native mode:
>> struct snd_pcm_status32 {
>>       ......
>>       struct { s32 tv_sec; s32 tv_nsec; } trigger_tstamp;
>>       struct { s32 tv_sec; s32 tv_nsec; } tstamp;
>>       ......
>> }
>>
>> struct snd_pcm_status64 {
>>       ......
>>       struct { s64 tv_sec; s64 tv_nsec; } trigger_tstamp;
>>       struct { s64 tv_sec; s64 tv_nsec; } tstamp;
>>       ......
>> }
>>
>> Moreover in compat file, we renamed or introduced new structures to handle
>> 32bit/64bit time_t in compatible mode. 'struct compat_snd_pcm_status32' and
>> snd_pcm_status_user_compat() are used to handle 32bit time_t in compat mode.
>> 'struct compat_snd_pcm_status64' and snd_pcm_status_user_compat64() are used
>> to handle 64bit time_t with 64bit alignment. 'struct compat_snd_pcm_status64_x86_32'
>> and snd_pcm_status_user_compat64_x86_32() are used to handle 64bit time_t with
>> 32bit alignment.
>
> Hmm...  I don't get why you need to redefine these in
> include/sound/pcm.h and define even IOCTLs there.  These are the
> things for ABI, no?  If yes, we don't need to have it globally in the
> public header but define/convert them locally.

The problem is that the ABI is currently defined in terms of a mix of data
structures from kernel and glibc. We have to be very careful about changing the
public header files to avoid breaking applications that were built against new
headers but run on old kernels, so we can't just make the time_t fields 64-bit
wide in the header and change the ioctl number for everyone.

The new structure definitions in the internal are representations of
the possible
binary layouts that user space will actually see with the possible combinations
of 32-bit and 64-bit toolchains, the x86-32 quirk (64-bit variables
being misaligned),
the x32 hack (makeing __kernel_long_t 64-bit) and new glibc using 64-bit time_t.

>  And, renaming snd_pcm_status64 allover the places for internal doesn't look good.

Would you prefer adding another distinct type for the kernel-internal structure?
That would probably be cleaner overall, but also complicate the one
case in which
the user-space representation is actually the same as the one in the kernel.
Note that we can't use snd_pcm_status here, because that is based on 'time_t',
which in the kernel has to remain defined as 'long', so we'd still suffer from
the overflow.

One more thing: as we discussed in Prague, I think the additional
compat_snd_pcm_status64_x86_32 structure can be avoided if we
make slight changes to the user-visible definition of snd_pcm_status
that only take effect on x86-32 with a modified libc, i.e.

diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
index 299a822d2c4e..40be757de124 100644
--- a/include/uapi/sound/asound.h
+++ b/include/uapi/sound/asound.h
@@ -447,6 +447,7 @@ enum {

 struct snd_pcm_status {
        snd_pcm_state_t state;          /* stream state */
+       __u8 __pad0[sizeof(time_t) - sizeof(__kernel_long_t)];
        struct timespec trigger_tstamp; /* time when stream was
started/stopped/paused */
        struct timespec tstamp;         /* reference timestamp */
        snd_pcm_uframes_t appl_ptr;     /* appl ptr */

With that change, some of the other changes should get simpler too.

      Arnd

[RFC PATCH v2 3/7] sound: core: Avoid using timespec for struct snd_rawmidi_status

[Baolin Wang]

The struct snd_rawmidi_status will use 'timespec' type variables to record
timestamp, which is not year 2038 safe on 32bits system.

Thus we introduced 'struct snd_rawmidi_status32' and 'struct snd_rawmidi_status64'
to handle 32bit time_t and 64bit time_t in native mode, which replace
timespec with s64 type.

In compat mode, we renamed or introduced new structures to handle 32bit/64bit
time_t in compatible mode. 'struct compat_snd_rawmidi_status32' and
snd_rawmidi_ioctl_status_compat() are used to handle 32bit time_t in compat mode.
'struct compat_snd_rawmidi_status64' and snd_rawmidi_ioctl_status_compat64() are used
to handle 64bit time_t with 64bit alignment. 'struct compat_snd_rawmidi_status64_x86_32'
and snd_rawmidi_ioctl_status_compat64_x86_32() are used to handle 64bit time_t with
32bit alignment.

When glibc changes time_t to 64-bit, any recompiled program will issue ioctl
commands that the kernel does not understand without this patch.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 sound/core/rawmidi.c        |   81 ++++++++++++++++++++++++++---
 sound/core/rawmidi_compat.c |  121 +++++++++++++++++++++++++++++++++----------
 2 files changed, 168 insertions(+), 34 deletions(-)

diff --git a/sound/core/rawmidi.c b/sound/core/rawmidi.c
index b3b353d..af79009 100644
--- a/sound/core/rawmidi.c
+++ b/sound/core/rawmidi.c
@@ -63,6 +63,30 @@
 #define rmidi_dbg(rmidi, fmt, args...) \
 	dev_dbg(&(rmidi)->dev, fmt, ##args)
 
+#if __BITS_PER_LONG == 32
+struct snd_rawmidi_status32 {
+	int stream;
+	s32 tstamp_sec;			/* Timestamp */
+	s32 tstamp_nsec;
+	size_t avail;			/* available bytes */
+	size_t xruns;			/* count of overruns since last status (in bytes) */
+	unsigned char reserved[16];	/* reserved for future use */
+};
+
+#define SNDRV_RAWMIDI_IOCTL_STATUS32	_IOWR('W', 0x20, struct snd_rawmidi_status32)
+#endif
+
+struct snd_rawmidi_status64 {
+	int stream;
+	s64 tstamp_sec;			/* Timestamp */
+	s64 tstamp_nsec;
+	size_t avail;			/* available bytes */
+	size_t xruns;			/* count of overruns since last status (in bytes) */
+	unsigned char reserved[16];	/* reserved for future use */
+};
+
+#define SNDRV_RAWMIDI_IOCTL_STATUS64	_IOWR('W', 0x20, struct snd_rawmidi_status64)
+
 static struct snd_rawmidi *snd_rawmidi_search(struct snd_card *card, int device)
 {
 	struct snd_rawmidi *rawmidi;
@@ -680,7 +704,7 @@ int snd_rawmidi_input_params(struct snd_rawmidi_substream *substream,
 EXPORT_SYMBOL(snd_rawmidi_input_params);
 
 static int snd_rawmidi_output_status(struct snd_rawmidi_substream *substream,
-				     struct snd_rawmidi_status * status)
+				     struct snd_rawmidi_status64 * status)
 {
 	struct snd_rawmidi_runtime *runtime = substream->runtime;
 
@@ -693,7 +717,7 @@ static int snd_rawmidi_output_status(struct snd_rawmidi_substream *substream,
 }
 
 static int snd_rawmidi_input_status(struct snd_rawmidi_substream *substream,
-				    struct snd_rawmidi_status * status)
+				    struct snd_rawmidi_status64 * status)
 {
 	struct snd_rawmidi_runtime *runtime = substream->runtime;
 
@@ -751,11 +775,55 @@ static long snd_rawmidi_ioctl(struct file *file, unsigned int cmd, unsigned long
 			return -EINVAL;
 		}
 	}
-	case SNDRV_RAWMIDI_IOCTL_STATUS:
+#if __BITS_PER_LONG == 32
+	case SNDRV_RAWMIDI_IOCTL_STATUS32:
+	{
+		int err = 0;
+		struct snd_rawmidi_status32 __user *status = argp;
+		struct snd_rawmidi_status32 status32;
+		struct snd_rawmidi_status64 status64;
+
+		if (copy_from_user(&status32, argp,
+				   sizeof(struct snd_rawmidi_status32)))
+			return -EFAULT;
+		switch (status32.stream) {
+		case SNDRV_RAWMIDI_STREAM_OUTPUT:
+			if (rfile->output == NULL)
+				return -EINVAL;
+			err = snd_rawmidi_output_status(rfile->output, &status64);
+			break;
+		case SNDRV_RAWMIDI_STREAM_INPUT:
+			if (rfile->input == NULL)
+				return -EINVAL;
+			err = snd_rawmidi_input_status(rfile->input, &status64);
+			break;
+		default:
+			return -EINVAL;
+		}
+		if (err < 0)
+			return err;
+
+		status32 = (struct snd_rawmidi_status32) {
+			.stream = status64.stream,
+			.tstamp_sec = status64.tstamp_sec,
+			.tstamp_nsec = status64.tstamp_nsec,
+			.avail = status64.avail,
+			.xruns = status64.xruns,
+		};
+
+		if (copy_to_user(status, &status32, sizeof(*status)))
+			return -EFAULT;
+
+		return 0;
+	}
+#endif
+	case SNDRV_RAWMIDI_IOCTL_STATUS64:
 	{
 		int err = 0;
-		struct snd_rawmidi_status status;
-		if (copy_from_user(&status, argp, sizeof(struct snd_rawmidi_status)))
+		struct snd_rawmidi_status64 status;
+
+		if (copy_from_user(&status, argp,
+				   sizeof(struct snd_rawmidi_status64)))
 			return -EFAULT;
 		switch (status.stream) {
 		case SNDRV_RAWMIDI_STREAM_OUTPUT:
@@ -773,7 +841,8 @@ static long snd_rawmidi_ioctl(struct file *file, unsigned int cmd, unsigned long
 		}
 		if (err < 0)
 			return err;
-		if (copy_to_user(argp, &status, sizeof(struct snd_rawmidi_status)))
+		if (copy_to_user(argp, &status,
+				 sizeof(struct snd_rawmidi_status64)))
 			return -EFAULT;
 		return 0;
 	}
diff --git a/sound/core/rawmidi_compat.c b/sound/core/rawmidi_compat.c
index f69764d..5b5b5ea 100644
--- a/sound/core/rawmidi_compat.c
+++ b/sound/core/rawmidi_compat.c
@@ -53,19 +53,21 @@ static int snd_rawmidi_ioctl_params_compat(struct snd_rawmidi_file *rfile,
 	return -EINVAL;
 }
 
-struct snd_rawmidi_status32 {
+struct compat_snd_rawmidi_status32 {
 	s32 stream;
-	struct compat_timespec tstamp;
+	s32 tstamp_sec;
+	s32 tstamp_nsec;
 	u32 avail;
 	u32 xruns;
 	unsigned char reserved[16];
 } __attribute__((packed));
 
 static int snd_rawmidi_ioctl_status_compat(struct snd_rawmidi_file *rfile,
-					   struct snd_rawmidi_status32 __user *src)
+					   struct compat_snd_rawmidi_status32 __user *src)
 {
 	int err;
-	struct snd_rawmidi_status status;
+	struct snd_rawmidi_status64 status;
+	struct compat_snd_rawmidi_status32 compat_status;
 
 	if (rfile->output == NULL)
 		return -EINVAL;
@@ -85,32 +87,86 @@ static int snd_rawmidi_ioctl_status_compat(struct snd_rawmidi_file *rfile,
 	if (err < 0)
 		return err;
 
-	if (compat_put_timespec(&status.tstamp, &src->tstamp) ||
-	    put_user(status.avail, &src->avail) ||
-	    put_user(status.xruns, &src->xruns))
+	compat_status = (struct compat_snd_rawmidi_status32) {
+		.stream = status.stream,
+		.tstamp_sec = status.tstamp_sec,
+		.tstamp_nsec = status.tstamp_nsec,
+		.avail = status.avail,
+		.xruns = status.xruns,
+	};
+
+	if (copy_to_user(src, &compat_status, sizeof(*src)))
 		return -EFAULT;
 
 	return 0;
 }
 
-#ifdef CONFIG_X86_X32
-/* X32 ABI has 64bit timespec and 64bit alignment */
-struct snd_rawmidi_status_x32 {
+struct compat_snd_rawmidi_status64 {
 	s32 stream;
 	u32 rsvd; /* alignment */
-	struct timespec tstamp;
+	s64 tstamp_sec;
+	s64 tstamp_nsec;
 	u32 avail;
 	u32 xruns;
 	unsigned char reserved[16];
 } __attribute__((packed));
 
-#define put_timespec(src, dst) copy_to_user(dst, src, sizeof(*dst))
+static int snd_rawmidi_ioctl_status_compat64(struct snd_rawmidi_file *rfile,
+					     struct compat_snd_rawmidi_status64 __user *src)
+{
+	int err;
+	struct snd_rawmidi_status64 status;
+	struct compat_snd_rawmidi_status64 compat_status;
+
+	if (rfile->output == NULL)
+		return -EINVAL;
+	if (get_user(status.stream, &src->stream))
+		return -EFAULT;
+
+	switch (status.stream) {
+	case SNDRV_RAWMIDI_STREAM_OUTPUT:
+		err = snd_rawmidi_output_status(rfile->output, &status);
+		break;
+	case SNDRV_RAWMIDI_STREAM_INPUT:
+		err = snd_rawmidi_input_status(rfile->input, &status);
+		break;
+	default:
+		return -EINVAL;
+	}
+	if (err < 0)
+		return err;
+
+	compat_status = (struct compat_snd_rawmidi_status64) {
+		.stream = status.stream,
+		.tstamp_sec = status.tstamp_sec,
+		.tstamp_nsec = status.tstamp_nsec,
+		.avail = status.avail,
+		.xruns = status.xruns,
+	};
 
-static int snd_rawmidi_ioctl_status_x32(struct snd_rawmidi_file *rfile,
-					struct snd_rawmidi_status_x32 __user *src)
+	if (copy_to_user(src, &compat_status, sizeof(*src)))
+		return -EFAULT;
+
+	return 0;
+}
+
+#ifdef IA32_EMULATION
+struct compat_snd_rawmidi_status64_x86_32 {
+	s32 stream;
+	s64 tstamp_sec;
+	s64 tstamp_nsec;
+	u32 avail;
+	u32 xruns;
+	unsigned char reserved[16];
+} __attribute__((packed));
+
+static int
+snd_rawmidi_ioctl_status_compat64_x86_32(struct snd_rawmidi_file *rfile,
+					 struct compat_snd_rawmidi_status64_x86_32 __user *src)
 {
 	int err;
-	struct snd_rawmidi_status status;
+	struct snd_rawmidi_status64 status;
+	struct compat_snd_rawmidi_status64_x86_32 compat_status;
 
 	if (rfile->output == NULL)
 		return -EINVAL;
@@ -130,21 +186,28 @@ static int snd_rawmidi_ioctl_status_x32(struct snd_rawmidi_file *rfile,
 	if (err < 0)
 		return err;
 
-	if (put_timespec(&status.tstamp, &src->tstamp) ||
-	    put_user(status.avail, &src->avail) ||
-	    put_user(status.xruns, &src->xruns))
+	compat_status = (struct compat_snd_rawmidi_status64_x86_32) {
+		.stream = status.stream,
+		.tstamp_sec = status.tstamp_sec,
+		.tstamp_nsec = status.tstamp_nsec,
+		.avail = status.avail,
+		.xruns = status.xruns,
+	};
+
+	if (copy_to_user(src, &compat_status, sizeof(*src)))
 		return -EFAULT;
 
 	return 0;
 }
-#endif /* CONFIG_X86_X32 */
+#endif
 
 enum {
 	SNDRV_RAWMIDI_IOCTL_PARAMS32 = _IOWR('W', 0x10, struct snd_rawmidi_params32),
-	SNDRV_RAWMIDI_IOCTL_STATUS32 = _IOWR('W', 0x20, struct snd_rawmidi_status32),
-#ifdef CONFIG_X86_X32
-	SNDRV_RAWMIDI_IOCTL_STATUS_X32 = _IOWR('W', 0x20, struct snd_rawmidi_status_x32),
-#endif /* CONFIG_X86_X32 */
+	SNDRV_RAWMIDI_IOCTL_STATUS_COMPAT32 = _IOWR('W', 0x20, struct compat_snd_rawmidi_status32),
+	SNDRV_RAWMIDI_IOCTL_STATUS_COMPAT64 = _IOWR('W', 0x20, struct compat_snd_rawmidi_status64),
+#ifdef IA32_EMULATION
+	SNDRV_RAWMIDI_IOCTL_STATUS_COMPAT64_X86_32 = _IOWR('W', 0x20, struct compat_snd_rawmidi_status64_x86_32),
+#endif
 };
 
 static long snd_rawmidi_ioctl_compat(struct file *file, unsigned int cmd, unsigned long arg)
@@ -161,12 +224,14 @@ static long snd_rawmidi_ioctl_compat(struct file *file, unsigned int cmd, unsign
 		return snd_rawmidi_ioctl(file, cmd, (unsigned long)argp);
 	case SNDRV_RAWMIDI_IOCTL_PARAMS32:
 		return snd_rawmidi_ioctl_params_compat(rfile, argp);
-	case SNDRV_RAWMIDI_IOCTL_STATUS32:
+	case SNDRV_RAWMIDI_IOCTL_STATUS_COMPAT32:
 		return snd_rawmidi_ioctl_status_compat(rfile, argp);
-#ifdef CONFIG_X86_X32
-	case SNDRV_RAWMIDI_IOCTL_STATUS_X32:
-		return snd_rawmidi_ioctl_status_x32(rfile, argp);
-#endif /* CONFIG_X86_X32 */
+	case SNDRV_RAWMIDI_IOCTL_STATUS_COMPAT64:
+		return snd_rawmidi_ioctl_status_compat64(rfile, argp);
+#ifdef IA32_EMULATION
+	case SNDRV_RAWMIDI_IOCTL_STATUS_COMPAT64_X86_32:
+		return snd_rawmidi_ioctl_status_compat64_x86_32(rfile, argp);
+#endif
 	}
 	return -ENOIOCTLCMD;
 }
-- 
1.7.9.5

[RFC PATCH v2 4/7] sound: core: Avoid using timespec for struct snd_timer_status

[Baolin Wang]

The struct snd_timer_status will use 'timespec' type variables to record
timestamp, which is not year 2038 safe on 32bits system.

Thus thia patch introduces 'struct snd_timer_status32' and 'struct snd_timer_status64'
to handle 32bit time_t and 64bit time_t in native mode, which replace
timespec with s64 type.

In compat mode, this patch renamed the structure as 'struct compat_snd_timer_status32'
to handle 32bit time_t. Moreover we should use 'struct snd_timer_status64'
to handle 64bit time_t no matter 32bit or 64bit alignment, since they have
the same size.

When glibc changes time_t to 64-bit, any recompiled program will issue ioctl
commands that the kernel does not understand without this patch.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 sound/core/timer.c        |   62 ++++++++++++++++++++++++++++++++++++++++-----
 sound/core/timer_compat.c |   57 +++++------------------------------------
 2 files changed, 62 insertions(+), 57 deletions(-)

diff --git a/sound/core/timer.c b/sound/core/timer.c
index f44d702..9168365 100644
--- a/sound/core/timer.c
+++ b/sound/core/timer.c
@@ -79,6 +79,30 @@ struct snd_timer_user {
 	struct mutex ioctl_lock;
 };
 
+struct snd_timer_status32 {
+	s32 tstamp_sec;			/* Timestamp - last update */
+	s32 tstamp_nsec;
+	unsigned int resolution;	/* current period resolution in ns */
+	unsigned int lost;		/* counter of master tick lost */
+	unsigned int overrun;		/* count of read queue overruns */
+	unsigned int queue;		/* used queue size */
+	unsigned char reserved[64];	/* reserved */
+};
+
+#define SNDRV_TIMER_IOCTL_STATUS32	_IOR('T', 0x14, struct snd_timer_status32)
+
+struct snd_timer_status64 {
+	s64 tstamp_sec;			/* Timestamp - last update */
+	s64 tstamp_nsec;
+	unsigned int resolution;	/* current period resolution in ns */
+	unsigned int lost;		/* counter of master tick lost */
+	unsigned int overrun;		/* count of read queue overruns */
+	unsigned int queue;		/* used queue size */
+	unsigned char reserved[64];	/* reserved */
+};
+
+#define SNDRV_TIMER_IOCTL_STATUS64	_IOR('T', 0x14, struct snd_timer_status64)
+
 /* list of timers */
 static LIST_HEAD(snd_timer_list);
 
@@ -1798,17 +1822,41 @@ static int snd_timer_user_params(struct file *file,
 	return err;
 }
 
-static int snd_timer_user_status(struct file *file,
-				 struct snd_timer_status __user *_status)
+static int snd_timer_user_status32(struct file *file,
+				   struct snd_timer_status32 __user *_status)
+ {
+	struct snd_timer_user *tu;
+	struct snd_timer_status32 status;
+
+	tu = file->private_data;
+	if (!tu->timeri)
+		return -EBADFD;
+	memset(&status, 0, sizeof(status));
+	status.tstamp_sec = tu->tstamp.tv_sec;
+	status.tstamp_nsec = tu->tstamp.tv_nsec;
+	status.resolution = snd_timer_resolution(tu->timeri);
+	status.lost = tu->timeri->lost;
+	status.overrun = tu->overrun;
+	spin_lock_irq(&tu->qlock);
+	status.queue = tu->qused;
+	spin_unlock_irq(&tu->qlock);
+	if (copy_to_user(_status, &status, sizeof(status)))
+		return -EFAULT;
+	return 0;
+}
+
+static int snd_timer_user_status64(struct file *file,
+				   struct snd_timer_status64 __user *_status)
 {
 	struct snd_timer_user *tu;
-	struct snd_timer_status status;
+	struct snd_timer_status64 status;
 
 	tu = file->private_data;
 	if (!tu->timeri)
 		return -EBADFD;
 	memset(&status, 0, sizeof(status));
-	status.tstamp = timespec64_to_timespec(tu->tstamp);
+	status.tstamp_sec = tu->tstamp.tv_sec;
+	status.tstamp_nsec = tu->tstamp.tv_nsec;
 	status.resolution = snd_timer_resolution(tu->timeri);
 	status.lost = tu->timeri->lost;
 	status.overrun = tu->overrun;
@@ -1920,8 +1968,10 @@ static long __snd_timer_user_ioctl(struct file *file, unsigned int cmd,
 		return snd_timer_user_info(file, argp);
 	case SNDRV_TIMER_IOCTL_PARAMS:
 		return snd_timer_user_params(file, argp);
-	case SNDRV_TIMER_IOCTL_STATUS:
-		return snd_timer_user_status(file, argp);
+	case SNDRV_TIMER_IOCTL_STATUS32:
+		return snd_timer_user_status32(file, argp);
+	case SNDRV_TIMER_IOCTL_STATUS64:
+		return snd_timer_user_status64(file, argp);
 	case SNDRV_TIMER_IOCTL_START:
 	case SNDRV_TIMER_IOCTL_START_OLD:
 		return snd_timer_user_start(file);
diff --git a/sound/core/timer_compat.c b/sound/core/timer_compat.c
index 6a437eb..3e09548 100644
--- a/sound/core/timer_compat.c
+++ b/sound/core/timer_compat.c
@@ -83,54 +83,11 @@ static int snd_timer_user_info_compat(struct file *file,
 	return 0;
 }
 
-struct snd_timer_status32 {
-	struct compat_timespec tstamp;
-	u32 resolution;
-	u32 lost;
-	u32 overrun;
-	u32 queue;
-	unsigned char reserved[64];
-};
-
-static int snd_timer_user_status_compat(struct file *file,
-					struct snd_timer_status32 __user *_status)
-{
-	struct snd_timer_user *tu;
-	struct snd_timer_status32 status;
-	
-	tu = file->private_data;
-	if (snd_BUG_ON(!tu->timeri))
-		return -ENXIO;
-	memset(&status, 0, sizeof(status));
-	status.tstamp.tv_sec = tu->tstamp.tv_sec;
-	status.tstamp.tv_nsec = tu->tstamp.tv_nsec;
-	status.resolution = snd_timer_resolution(tu->timeri);
-	status.lost = tu->timeri->lost;
-	status.overrun = tu->overrun;
-	spin_lock_irq(&tu->qlock);
-	status.queue = tu->qused;
-	spin_unlock_irq(&tu->qlock);
-	if (copy_to_user(_status, &status, sizeof(status)))
-		return -EFAULT;
-	return 0;
-}
-
-#ifdef CONFIG_X86_X32
-/* X32 ABI has the same struct as x86-64 */
-#define snd_timer_user_status_x32(file, s) \
-	snd_timer_user_status(file, s)
-#endif /* CONFIG_X86_X32 */
-
-/*
- */
-
 enum {
 	SNDRV_TIMER_IOCTL_GPARAMS32 = _IOW('T', 0x04, struct snd_timer_gparams32),
 	SNDRV_TIMER_IOCTL_INFO32 = _IOR('T', 0x11, struct snd_timer_info32),
-	SNDRV_TIMER_IOCTL_STATUS32 = _IOW('T', 0x14, struct snd_timer_status32),
-#ifdef CONFIG_X86_X32
-	SNDRV_TIMER_IOCTL_STATUS_X32 = _IOW('T', 0x14, struct snd_timer_status),
-#endif /* CONFIG_X86_X32 */
+	SNDRV_TIMER_IOCTL_STATUS_COMPAT32 = _IOW('T', 0x14, struct snd_timer_status32),
+	SNDRV_TIMER_IOCTL_STATUS_COMPAT64 = _IOW('T', 0x14, struct snd_timer_status64),
 };
 
 static long snd_timer_user_ioctl_compat(struct file *file, unsigned int cmd, unsigned long arg)
@@ -158,12 +115,10 @@ static long snd_timer_user_ioctl_compat(struct file *file, unsigned int cmd, uns
 		return snd_timer_user_gparams_compat(file, argp);
 	case SNDRV_TIMER_IOCTL_INFO32:
 		return snd_timer_user_info_compat(file, argp);
-	case SNDRV_TIMER_IOCTL_STATUS32:
-		return snd_timer_user_status_compat(file, argp);
-#ifdef CONFIG_X86_X32
-	case SNDRV_TIMER_IOCTL_STATUS_X32:
-		return snd_timer_user_status_x32(file, argp);
-#endif /* CONFIG_X86_X32 */
+	case SNDRV_TIMER_IOCTL_STATUS_COMPAT32:
+		return snd_timer_user_status32(file, argp);
+	case SNDRV_TIMER_IOCTL_STATUS_COMPAT64:
+		return snd_timer_user_status64(file, argp);
 	}
 	return -ENOIOCTLCMD;
 }
-- 
1.7.9.5

[RFC PATCH v2 5/7] uapi: sound: Avoid using timespec for struct snd_ctl_elem_value

[Baolin Wang]

The struct snd_ctl_elem_value will use 'timespec' type variables to record
timestamp, which is not year 2038 safe on 32bits system.

Since there are no drivers will implemented the tstamp member of the
struct snd_ctl_elem_value, and also the stucture size will not be changed
if we change timespec to s64 for tstamp member of struct snd_ctl_elem_value.

Thus we can simply change timespec to s64 for tstamp member to avoid
using the type which is not year 2038 safe on 32bits system.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 include/uapi/sound/asound.h |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
index 1949923..fabb283 100644
--- a/include/uapi/sound/asound.h
+++ b/include/uapi/sound/asound.h
@@ -943,8 +943,12 @@ struct snd_ctl_elem_value {
 		} bytes;
 		struct snd_aes_iec958 iec958;
 	} value;		/* RO */
-	struct timespec tstamp;
-	unsigned char reserved[128-sizeof(struct timespec)];
+#ifndef __KERNEL__
+	struct { s64 tv_sec; s64 tv_nsec; } tstamp;
+	unsigned char reserved[128-sizeof(struct { s64 tv_sec; s64 tv_nsec; })];
+#else
+	unsigned char reserved[128];
+#endif
 };
 
 struct snd_ctl_tlv {
-- 
1.7.9.5

Re: [RFC PATCH v2 5/7] uapi: sound: Avoid using timespec for struct snd_ctl_elem_value

[Takashi Sakamoto]

Hi,

On Nov 2 2017 20:06, Baolin Wang wrote:
> The struct snd_ctl_elem_value will use 'timespec' type variables to record
> timestamp, which is not year 2038 safe on 32bits system.
> 
> Since there are no drivers will implemented the tstamp member of the
> struct snd_ctl_elem_value, and also the stucture size will not be changed
> if we change timespec to s64 for tstamp member of struct snd_ctl_elem_value.
> 
> Thus we can simply change timespec to s64 for tstamp member to avoid
> using the type which is not year 2038 safe on 32bits system.
> 
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---
>   include/uapi/sound/asound.h |    8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
> index 1949923..fabb283 100644
> --- a/include/uapi/sound/asound.h
> +++ b/include/uapi/sound/asound.h
> @@ -943,8 +943,12 @@ struct snd_ctl_elem_value {
>   		} bytes;
>   		struct snd_aes_iec958 iec958;
>   	} value;		/* RO */
> -	struct timespec tstamp;
> -	unsigned char reserved[128-sizeof(struct timespec)];
> +#ifndef __KERNEL__
> +	struct { s64 tv_sec; s64 tv_nsec; } tstamp;
> +	unsigned char reserved[128-sizeof(struct { s64 tv_sec; s64 tv_nsec; })];
> +#else
> +	unsigned char reserved[128];
> +#endif
>   };
>   
>   struct snd_ctl_tlv {

As long as I know, via any APIs in alsa-lib[1], 'struct 
snd_ctl_elem_value.tstamp' is not available, fortunately.

In the library, applications are not expected to access to this 
structure directly. The applications get opaque pointer to the structure 
and must use any control APIs to operate it. Actually the library 
produce no API to handle 'struct snd_ctl_elem_value.tstamp'. This means 
that we can drop this member from alsa-lib without decline of functionality.

As you know, the member is abandoned in kernel side as well. This allows 
us to judge that this feature is not practically used by any userspace 
implementations such as tinyalsa[2].

In my opinion, we can have a plan to drop this useless member instead of 
this patch. Of course, we should have enough investigation and 
consideration about its meaning on ALSA control interface in advance of 
actual removal.

[1] http://git.alsa-project.org/?p=alsa-lib.git
[2] https://android.googlesource.com/platform/external/tinyalsa/


Thanks

Takashi Sakamoto

[RFC PATCH v2 6/7] sound: core: Avoid using timespec for struct snd_pcm_sync_ptr

[Baolin Wang]

The struct snd_pcm_sync_ptr will use 'timespec' type variables to record
timestamp, which is not year 2038 safe on 32bits system.

Thus we introduced 'struct snd_pcm_sync_ptr32' and 'struct snd_pcm_sync_ptr64'
to handle 32bit time_t and 64bit time_t in native mode, which replace
timespec with s64 type.

In compat mode, we renamed or introduced new structures to handle 32bit/64bit
time_t in compatible mode. The 'struct compat_snd_pcm_sync_ptr32' and
snd_pcm_ioctl_sync_ptr_compat() are used to handle 32bit time_t in compat mode.
'struct compat_snd_pcm_sync_ptr64' and snd_pcm_ioctl_sync_ptr_compat64() are used
to handle 64bit time_t with 64bit alignment. 'struct compat_snd_pcm_sync_ptr64_x86_32'
and snd_pcm_ioctl_sync_ptr_compat64_x86_32() are used to handle 64bit time_t with
32bit alignment.

When glibc changes time_t to 64bit, any recompiled program will issue ioctl
commands that the kernel does not understand without this patch.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 include/sound/pcm.h     |   51 ++++++++++-
 sound/core/pcm.c        |    8 +-
 sound/core/pcm_compat.c |  233 ++++++++++++++++++++++++++++++++++++-----------
 sound/core/pcm_lib.c    |    9 +-
 sound/core/pcm_native.c |  122 +++++++++++++++++++------
 5 files changed, 336 insertions(+), 87 deletions(-)

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 7524b54..b23b4f5 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -64,6 +64,18 @@ struct snd_pcm_hardware {
 struct snd_pcm_audio_tstamp_config; /* definitions further down */
 struct snd_pcm_audio_tstamp_report;
 
+struct snd_pcm_mmap_status64 {
+	snd_pcm_state_t state;		/* RO: state - SNDRV_PCM_STATE_XXXX */
+	int pad1;			/* Needed for 64 bit alignment */
+	u64 hw_ptr;			/* RO: hw ptr (0...boundary-1) */
+	s64 tstamp_sec;			/* Timestamp */
+	s64 tstamp_nsec;
+	snd_pcm_state_t suspended_state; /* RO: suspended stream state */
+	u32 pad3;
+	s64 audio_tstamp_sec;		/* from sample counter or wall clock */
+	s64 audio_tstamp_nsec;
+};
+
 struct snd_pcm_ops {
 	int (*open)(struct snd_pcm_substream *substream);
 	int (*close)(struct snd_pcm_substream *substream);
@@ -389,7 +401,7 @@ struct snd_pcm_runtime {
 	union snd_pcm_sync_id sync;	/* hardware synchronization ID */
 
 	/* -- mmap -- */
-	struct snd_pcm_mmap_status *status;
+	struct snd_pcm_mmap_status64 *status;
 	struct snd_pcm_mmap_control *control;
 
 	/* -- locking / scheduling -- */
@@ -1463,8 +1475,32 @@ struct snd_pcm_status64 {
 	unsigned char reserved[52-4*sizeof(s64)]; /* must be filled with zero */
 };
 
+struct snd_pcm_sync_ptr64 {
+	unsigned int flags;
+	union {
+		struct snd_pcm_mmap_status64 status;
+		unsigned char reserved[64];
+	} s;
+	union {
+		struct snd_pcm_mmap_control control;
+		unsigned char reserved[64];
+	} c;
+};
+
+struct snd_pcm_mmap_status32 {
+	snd_pcm_state_t state;		/* RO: state - SNDRV_PCM_STATE_XXXX */
+	int pad1;			/* Needed for 64 bit alignment */
+	u32 hw_ptr;			/* RO: hw ptr (0...boundary-1) */
+	s32 tstamp_sec;			/* Timestamp */
+	s32 tstamp_nsec;
+	snd_pcm_state_t suspended_state; /* RO: suspended stream state */
+	s32 audio_tstamp_sec;		/* from sample counter or wall clock */
+	s32 audio_tstamp_nsec;
+};
+
 #define SNDRV_PCM_IOCTL_STATUS64	_IOR('A', 0x20, struct snd_pcm_status64)
 #define SNDRV_PCM_IOCTL_STATUS_EXT64	_IOWR('A', 0x24, struct snd_pcm_status64)
+#define SNDRV_PCM_IOCTL_SYNC_PTR64	_IOWR('A', 0x23, struct snd_pcm_sync_ptr64)
 
 #if __BITS_PER_LONG == 32
 struct snd_pcm_status32 {
@@ -1489,8 +1525,21 @@ struct snd_pcm_status32 {
 	unsigned char reserved[52-4*sizeof(s32)]; /* must be filled with zero */
 };
 
+struct snd_pcm_sync_ptr32 {
+	unsigned int flags;
+	union {
+		struct snd_pcm_mmap_status32 status;
+		unsigned char reserved[64];
+	} s;
+	union {
+		struct snd_pcm_mmap_control control;
+		unsigned char reserved[64];
+	} c;
+};
+
 #define SNDRV_PCM_IOCTL_STATUS32	_IOR('A', 0x20, struct snd_pcm_status32)
 #define SNDRV_PCM_IOCTL_STATUS_EXT32	_IOWR('A', 0x24, struct snd_pcm_status32)
+#define SNDRV_PCM_IOCTL_SYNC_PTR32	_IOWR('A', 0x23, struct snd_pcm_sync_ptr32)
 #endif
 
 #endif /* __SOUND_PCM_H */
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index c19e656..3c27e68 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -478,7 +478,7 @@ static void snd_pcm_substream_proc_status_read(struct snd_info_entry *entry,
 	snd_iprintf(buffer, "avail       : %ld\n", status.avail);
 	snd_iprintf(buffer, "avail_max   : %ld\n", status.avail_max);
 	snd_iprintf(buffer, "-----\n");
-	snd_iprintf(buffer, "hw_ptr      : %ld\n", runtime->status->hw_ptr);
+	snd_iprintf(buffer, "hw_ptr      : %lld\n", runtime->status->hw_ptr);
 	snd_iprintf(buffer, "appl_ptr    : %ld\n", runtime->control->appl_ptr);
  unlock:
 	mutex_unlock(&substream->pcm->open_mutex);
@@ -1001,7 +1001,7 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream,
 	if (runtime == NULL)
 		return -ENOMEM;
 
-	size = PAGE_ALIGN(sizeof(struct snd_pcm_mmap_status));
+	size = PAGE_ALIGN(sizeof(struct snd_pcm_mmap_status64));
 	runtime->status = snd_malloc_pages(size, GFP_KERNEL);
 	if (runtime->status == NULL) {
 		kfree(runtime);
@@ -1013,7 +1013,7 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream,
 	runtime->control = snd_malloc_pages(size, GFP_KERNEL);
 	if (runtime->control == NULL) {
 		snd_free_pages((void*)runtime->status,
-			       PAGE_ALIGN(sizeof(struct snd_pcm_mmap_status)));
+			       PAGE_ALIGN(sizeof(struct snd_pcm_mmap_status64)));
 		kfree(runtime);
 		return -ENOMEM;
 	}
@@ -1044,7 +1044,7 @@ void snd_pcm_detach_substream(struct snd_pcm_substream *substream)
 	if (runtime->private_free != NULL)
 		runtime->private_free(runtime);
 	snd_free_pages((void*)runtime->status,
-		       PAGE_ALIGN(sizeof(struct snd_pcm_mmap_status)));
+		       PAGE_ALIGN(sizeof(struct snd_pcm_mmap_status64)));
 	snd_free_pages((void*)runtime->control,
 		       PAGE_ALIGN(sizeof(struct snd_pcm_mmap_control)));
 	kfree(runtime->hw_constraints.rules);
diff --git a/sound/core/pcm_compat.c b/sound/core/pcm_compat.c
index 53b83d4..885aefb 100644
--- a/sound/core/pcm_compat.c
+++ b/sound/core/pcm_compat.c
@@ -557,42 +557,33 @@ static int snd_pcm_ioctl_xfern_compat(struct snd_pcm_substream *substream,
 	return err;
 }
 
-
-struct snd_pcm_mmap_status32 {
-	s32 state;
-	s32 pad1;
-	u32 hw_ptr;
-	struct compat_timespec tstamp;
-	s32 suspended_state;
-	struct compat_timespec audio_tstamp;
-} __attribute__((packed));
-
-struct snd_pcm_mmap_control32 {
+struct compat_snd_pcm_mmap_control32 {
 	u32 appl_ptr;
 	u32 avail_min;
 };
 
-struct snd_pcm_sync_ptr32 {
+struct compat_snd_pcm_sync_ptr32 {
 	u32 flags;
 	union {
 		struct snd_pcm_mmap_status32 status;
 		unsigned char reserved[64];
 	} s;
 	union {
-		struct snd_pcm_mmap_control32 control;
+		struct compat_snd_pcm_mmap_control32 control;
 		unsigned char reserved[64];
 	} c;
 } __attribute__((packed));
 
 static int snd_pcm_ioctl_sync_ptr_compat(struct snd_pcm_substream *substream,
-					 struct snd_pcm_sync_ptr32 __user *src)
+					 struct compat_snd_pcm_sync_ptr32 __user *src)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	volatile struct snd_pcm_mmap_status *status;
+	volatile struct snd_pcm_mmap_status64 *status;
 	volatile struct snd_pcm_mmap_control *control;
 	u32 sflags;
 	struct snd_pcm_mmap_control scontrol;
-	struct snd_pcm_mmap_status sstatus;
+	struct snd_pcm_mmap_status64 sstatus;
+	struct compat_snd_pcm_sync_ptr32 sync_ptr;
 	snd_pcm_uframes_t boundary;
 	int err;
 
@@ -625,63 +616,76 @@ static int snd_pcm_ioctl_sync_ptr_compat(struct snd_pcm_substream *substream,
 		scontrol.avail_min = control->avail_min;
 	sstatus.state = status->state;
 	sstatus.hw_ptr = status->hw_ptr % boundary;
-	sstatus.tstamp = status->tstamp;
+	sstatus.tstamp_sec = status->tstamp_sec;
+	sstatus.tstamp_nsec = status->tstamp_nsec;
 	sstatus.suspended_state = status->suspended_state;
-	sstatus.audio_tstamp = status->audio_tstamp;
+	sstatus.audio_tstamp_sec = status->audio_tstamp_sec;
+	sstatus.audio_tstamp_nsec = status->audio_tstamp_nsec;
 	snd_pcm_stream_unlock_irq(substream);
-	if (put_user(sstatus.state, &src->s.status.state) ||
-	    put_user(sstatus.hw_ptr, &src->s.status.hw_ptr) ||
-	    compat_put_timespec(&sstatus.tstamp, &src->s.status.tstamp) ||
-	    put_user(sstatus.suspended_state, &src->s.status.suspended_state) ||
-	    compat_put_timespec(&sstatus.audio_tstamp,
-		    &src->s.status.audio_tstamp) ||
-	    put_user(scontrol.appl_ptr, &src->c.control.appl_ptr) ||
-	    put_user(scontrol.avail_min, &src->c.control.avail_min))
+
+	memset(&sync_ptr, 0, sizeof(sync_ptr));
+	sync_ptr.s.status = (struct snd_pcm_mmap_status32) {
+		.state = sstatus.state,
+		.hw_ptr = sstatus.hw_ptr,
+		.tstamp_sec = sstatus.tstamp_sec,
+		.tstamp_nsec = sstatus.tstamp_nsec,
+		.suspended_state = sstatus.suspended_state,
+		.audio_tstamp_sec = sstatus.audio_tstamp_sec,
+		.audio_tstamp_nsec = sstatus.audio_tstamp_nsec,
+	};
+
+	sync_ptr.c.control = (struct compat_snd_pcm_mmap_control32) {
+		.appl_ptr = scontrol.appl_ptr,
+		.avail_min = scontrol.avail_min,
+	};
+
+	if (copy_to_user(src, &sync_ptr, sizeof(sync_ptr)))
 		return -EFAULT;
 
 	return 0;
 }
 
-#ifdef CONFIG_X86_X32
-/* X32 ABI has 64bit timespec and 64bit alignment */
-struct snd_pcm_mmap_status_x32 {
+struct compat_snd_pcm_mmap_status64 {
 	s32 state;
 	s32 pad1;
 	u32 hw_ptr;
 	u32 pad2; /* alignment */
-	struct timespec tstamp;
+	s64 tstamp_sec;
+	s64 tstamp_nsec;
 	s32 suspended_state;
 	s32 pad3;
-	struct timespec audio_tstamp;
+	s64 audio_tstamp_sec;
+	s64 audio_tstamp_nsec;
 } __packed;
 
-struct snd_pcm_mmap_control_x32 {
+struct compat_snd_pcm_mmap_control64 {
 	u32 appl_ptr;
 	u32 avail_min;
 };
 
-struct snd_pcm_sync_ptr_x32 {
+struct compat_snd_pcm_sync_ptr64 {
 	u32 flags;
 	u32 rsvd; /* alignment */
 	union {
-		struct snd_pcm_mmap_status_x32 status;
+		struct compat_snd_pcm_mmap_status64 status;
 		unsigned char reserved[64];
 	} s;
 	union {
-		struct snd_pcm_mmap_control_x32 control;
+		struct compat_snd_pcm_mmap_control64 control;
 		unsigned char reserved[64];
 	} c;
 } __packed;
 
-static int snd_pcm_ioctl_sync_ptr_x32(struct snd_pcm_substream *substream,
-				      struct snd_pcm_sync_ptr_x32 __user *src)
+static int snd_pcm_ioctl_sync_ptr_compat64(struct snd_pcm_substream *substream,
+				struct compat_snd_pcm_sync_ptr64 __user *src)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	volatile struct snd_pcm_mmap_status *status;
+	volatile struct snd_pcm_mmap_status64 *status;
 	volatile struct snd_pcm_mmap_control *control;
 	u32 sflags;
 	struct snd_pcm_mmap_control scontrol;
-	struct snd_pcm_mmap_status sstatus;
+	struct snd_pcm_mmap_status64 sstatus;
+	struct compat_snd_pcm_sync_ptr64 sync_ptr;
 	snd_pcm_uframes_t boundary;
 	int err;
 
@@ -714,22 +718,136 @@ static int snd_pcm_ioctl_sync_ptr_x32(struct snd_pcm_substream *substream,
 		scontrol.avail_min = control->avail_min;
 	sstatus.state = status->state;
 	sstatus.hw_ptr = status->hw_ptr % boundary;
-	sstatus.tstamp = status->tstamp;
+	sstatus.tstamp_sec = status->tstamp_sec;
+	sstatus.tstamp_nsec = status->tstamp_nsec;
 	sstatus.suspended_state = status->suspended_state;
-	sstatus.audio_tstamp = status->audio_tstamp;
+	sstatus.audio_tstamp_sec = status->audio_tstamp_sec;
+	sstatus.audio_tstamp_nsec = status->audio_tstamp_nsec;
 	snd_pcm_stream_unlock_irq(substream);
-	if (put_user(sstatus.state, &src->s.status.state) ||
-	    put_user(sstatus.hw_ptr, &src->s.status.hw_ptr) ||
-	    put_timespec(&sstatus.tstamp, &src->s.status.tstamp) ||
-	    put_user(sstatus.suspended_state, &src->s.status.suspended_state) ||
-	    put_timespec(&sstatus.audio_tstamp, &src->s.status.audio_tstamp) ||
-	    put_user(scontrol.appl_ptr, &src->c.control.appl_ptr) ||
-	    put_user(scontrol.avail_min, &src->c.control.avail_min))
+
+	memset(&sync_ptr, 0, sizeof(sync_ptr));
+	sync_ptr.s.status = (struct compat_snd_pcm_mmap_status64) {
+		.state = sstatus.state,
+		.hw_ptr = sstatus.hw_ptr,
+		.tstamp_sec = sstatus.tstamp_sec,
+		.tstamp_nsec = sstatus.tstamp_nsec,
+		.suspended_state = sstatus.suspended_state,
+		.audio_tstamp_sec = sstatus.audio_tstamp_sec,
+		.audio_tstamp_nsec = sstatus.audio_tstamp_nsec,
+	};
+
+	sync_ptr.c.control = (struct compat_snd_pcm_mmap_control64) {
+		.appl_ptr = scontrol.appl_ptr,
+		.avail_min = scontrol.avail_min,
+	};
+
+	if (copy_to_user(src, &sync_ptr, sizeof(sync_ptr)))
 		return -EFAULT;
 
 	return 0;
 }
-#endif /* CONFIG_X86_X32 */
+
+#ifdef IA32_EMULATION
+struct compat_snd_pcm_mmap_status64_x86_32 {
+	s32 state;
+	s32 pad1;
+	u32 hw_ptr;
+	s64 tstamp_sec;
+	s64 tstamp_nsec;
+	s32 suspended_state;
+	s64 audio_tstamp_sec;
+	s64 audio_tstamp_nsec;
+} __packed;
+
+struct compat_snd_pcm_mmap_control64_x86_32 {
+	u32 appl_ptr;
+	u32 avail_min;
+};
+
+struct compat_snd_pcm_sync_ptr64_x86_32 {
+	u32 flags;
+	union {
+		struct compat_snd_pcm_mmap_status64_x86_32 status;
+		unsigned char reserved[64];
+	} s;
+	union {
+		struct compat_snd_pcm_mmap_control64_x86_32 control;
+		unsigned char reserved[64];
+	} c;
+} __packed;
+
+static int
+snd_pcm_ioctl_sync_ptr_compat64_x86_32(struct snd_pcm_substream *substream,
+			struct compat_snd_pcm_sync_ptr64_x86_32 __user *src)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	volatile struct snd_pcm_mmap_status64 *status;
+	volatile struct snd_pcm_mmap_control *control;
+	u32 sflags;
+	struct snd_pcm_mmap_control scontrol;
+	struct snd_pcm_mmap_status64 sstatus;
+	struct compat_snd_pcm_sync_ptr64_x86_32 sync_ptr;
+	snd_pcm_uframes_t boundary;
+	int err;
+
+	if (snd_BUG_ON(!runtime))
+		return -EINVAL;
+
+	if (get_user(sflags, &src->flags) ||
+	    get_user(scontrol.appl_ptr, &src->c.control.appl_ptr) ||
+	    get_user(scontrol.avail_min, &src->c.control.avail_min))
+		return -EFAULT;
+	if (sflags & SNDRV_PCM_SYNC_PTR_HWSYNC) {
+		err = snd_pcm_hwsync(substream);
+		if (err < 0)
+			return err;
+	}
+	status = runtime->status;
+	control = runtime->control;
+	boundary = recalculate_boundary(runtime);
+	if (!boundary)
+		boundary = 0x7fffffff;
+	snd_pcm_stream_lock_irq(substream);
+	/* FIXME: we should consider the boundary for the sync from app */
+	if (!(sflags & SNDRV_PCM_SYNC_PTR_APPL))
+		control->appl_ptr = scontrol.appl_ptr;
+	else
+		scontrol.appl_ptr = control->appl_ptr % boundary;
+	if (!(sflags & SNDRV_PCM_SYNC_PTR_AVAIL_MIN))
+		control->avail_min = scontrol.avail_min;
+	else
+		scontrol.avail_min = control->avail_min;
+	sstatus.state = status->state;
+	sstatus.hw_ptr = status->hw_ptr % boundary;
+	sstatus.tstamp_sec = status->tstamp_sec;
+	sstatus.tstamp_nsec = status->tstamp_nsec;
+	sstatus.suspended_state = status->suspended_state;
+	sstatus.audio_tstamp_sec = status->audio_tstamp_sec;
+	sstatus.audio_tstamp_nsec = status->audio_tstamp_nsec;
+	snd_pcm_stream_unlock_irq(substream);
+
+	memset(&sync_ptr, 0, sizeof(sync_ptr));
+	sync_ptr.s.status = (struct compat_snd_pcm_mmap_status64_x86_32) {
+		.state = sstatus.state,
+		.hw_ptr = sstatus.hw_ptr,
+		.tstamp_sec = sstatus.tstamp_sec,
+		.tstamp_nsec = sstatus.tstamp_nsec,
+		.suspended_state = sstatus.suspended_state,
+		.audio_tstamp_sec = sstatus.audio_tstamp_sec,
+		.audio_tstamp_nsec = sstatus.audio_tstamp_nsec,
+	};
+
+	sync_ptr.c.control = (struct compat_snd_pcm_mmap_control64_x86_32) {
+		.appl_ptr = scontrol.appl_ptr,
+		.avail_min = scontrol.avail_min,
+	};
+
+	if (copy_to_user(src, &sync_ptr, sizeof(sync_ptr)))
+		return -EFAULT;
+
+	return 0;
+}
+#endif
 
 /*
  */
@@ -747,12 +865,14 @@ enum {
 	SNDRV_PCM_IOCTL_READI_FRAMES32 = _IOR('A', 0x51, struct snd_xferi32),
 	SNDRV_PCM_IOCTL_WRITEN_FRAMES32 = _IOW('A', 0x52, struct snd_xfern32),
 	SNDRV_PCM_IOCTL_READN_FRAMES32 = _IOR('A', 0x53, struct snd_xfern32),
-	SNDRV_PCM_IOCTL_SYNC_PTR32 = _IOWR('A', 0x23, struct snd_pcm_sync_ptr32),
+	SNDRV_PCM_IOCTL_SYNC_PTR_COMPAT32 = _IOWR('A', 0x23, struct compat_snd_pcm_sync_ptr32),
+	SNDRV_PCM_IOCTL_SYNC_PTR_COMPAT64 = _IOWR('A', 0x23, struct compat_snd_pcm_sync_ptr64),
 	SNDRV_PCM_IOCTL_STATUS_COMPAT64 = _IOR('A', 0x20, struct compat_snd_pcm_status64),
 	SNDRV_PCM_IOCTL_STATUS_EXT_COMPAT64 = _IOWR('A', 0x24, struct compat_snd_pcm_status64),
 #ifdef IA32_EMULATION
 	SNDRV_PCM_IOCTL_STATUS_COMPAT64_X86_32 = _IOR('A', 0x20, struct compat_snd_pcm_status64_x86_32),
 	SNDRV_PCM_IOCTL_STATUS_EXT_COMPAT64_X86_32 = _IOWR('A', 0x24, struct compat_snd_pcm_status64_x86_32),
+	SNDRV_PCM_IOCTL_SYNC_PTR_COMPAT64_X86_32 = _IOWR('A', 0x23, struct compat_snd_pcm_sync_ptr64_x86_32),
 #endif
 #ifdef CONFIG_X86_X32
 	SNDRV_PCM_IOCTL_CHANNEL_INFO_X32 = _IOR('A', 0x32, struct snd_pcm_channel_info),
@@ -809,8 +929,17 @@ static long snd_pcm_ioctl_compat(struct file *file, unsigned int cmd, unsigned l
 		return snd_pcm_status_user_compat(substream, argp, false);
 	case SNDRV_PCM_IOCTL_STATUS_EXT_COMPAT32:
 		return snd_pcm_status_user_compat(substream, argp, true);
-	case SNDRV_PCM_IOCTL_SYNC_PTR32:
-		return snd_pcm_ioctl_sync_ptr_compat(substream, argp);
+	case SNDRV_PCM_IOCTL_SYNC_PTR_COMPAT32:
+	{
+		if (sizeof(__kernel_long_t) == sizeof(time_t))
+			return snd_pcm_ioctl_sync_ptr_compat(substream, argp);
+#ifdef IA32_EMULATION
+		else
+			return snd_pcm_ioctl_sync_ptr_compat64_x86_32(substream, argp);
+#endif
+	}
+	case SNDRV_PCM_IOCTL_SYNC_PTR_COMPAT64:
+		return snd_pcm_ioctl_sync_ptr_compat64(substream, argp);
 	case SNDRV_PCM_IOCTL_CHANNEL_INFO32:
 		return snd_pcm_ioctl_channel_info_compat(substream, argp);
 	case SNDRV_PCM_IOCTL_WRITEI_FRAMES32:
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index 5ca9dc3..f3e5d43 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -162,7 +162,8 @@ static void xrun(struct snd_pcm_substream *substream)
 		struct timespec64 tstamp;
 
 		snd_pcm_gettime(runtime, &tstamp);
-		runtime->status->tstamp = timespec64_to_timespec(tstamp);
+		runtime->status->tstamp_sec = tstamp.tv_sec;
+		runtime->status->tstamp_nsec = tstamp.tv_nsec;
 	}
 	snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN);
 	if (xrun_debug(substream, XRUN_DEBUG_BASIC)) {
@@ -252,8 +253,10 @@ static void update_audio_tstamp(struct snd_pcm_substream *substream,
 				runtime->rate);
 		*audio_tstamp = ns_to_timespec64(audio_nsecs);
 	}
-	runtime->status->audio_tstamp = timespec64_to_timespec(*audio_tstamp);
-	runtime->status->tstamp = timespec64_to_timespec(*curr_tstamp);
+	runtime->status->audio_tstamp_sec = audio_tstamp->tv_sec;
+	runtime->status->audio_tstamp_nsec = audio_tstamp->tv_nsec;
+	runtime->status->tstamp_sec = curr_tstamp->tv_sec;
+	runtime->status->tstamp_nsec = curr_tstamp->tv_nsec;
 
 	/*
 	 * re-take a driver timestamp to let apps detect if the reference tstamp
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 7a70848..6063522 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -886,17 +886,17 @@ int snd_pcm_status64(struct snd_pcm_substream *substream,
 	if (snd_pcm_running(substream)) {
 		snd_pcm_update_hw_ptr(substream);
 		if (runtime->tstamp_mode == SNDRV_PCM_TSTAMP_ENABLE) {
-			status->tstamp_sec = runtime->status->tstamp.tv_sec;
+			status->tstamp_sec = runtime->status->tstamp_sec;
 			status->tstamp_nsec =
-				runtime->status->tstamp.tv_nsec;
+				runtime->status->tstamp_nsec;
 			status->driver_tstamp_sec =
 				runtime->driver_tstamp.tv_sec;
 			status->driver_tstamp_nsec =
 				runtime->driver_tstamp.tv_nsec;
 			status->audio_tstamp_sec =
-				runtime->status->audio_tstamp.tv_sec;
+				runtime->status->audio_tstamp_sec;
 			status->audio_tstamp_nsec =
-				runtime->status->audio_tstamp.tv_nsec;
+				runtime->status->audio_tstamp_nsec;
 			if (runtime->audio_tstamp_report.valid == 1)
 				/* backwards compatibility, no report provided in COMPAT mode */
 				snd_pcm_pack_audio_tstamp_report(&status->audio_tstamp_data,
@@ -2766,50 +2766,109 @@ static snd_pcm_sframes_t snd_pcm_delay(struct snd_pcm_substream *substream)
 	snd_pcm_stream_unlock_irq(substream);
 	return err < 0 ? err : n;
 }
-		
+
 static int snd_pcm_sync_ptr(struct snd_pcm_substream *substream,
-			    struct snd_pcm_sync_ptr __user *_sync_ptr)
+			    struct snd_pcm_sync_ptr64 *sync_ptr)
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	struct snd_pcm_sync_ptr sync_ptr;
-	volatile struct snd_pcm_mmap_status *status;
+	volatile struct snd_pcm_mmap_status64 *status;
 	volatile struct snd_pcm_mmap_control *control;
 	int err;
 
-	memset(&sync_ptr, 0, sizeof(sync_ptr));
-	if (get_user(sync_ptr.flags, (unsigned __user *)&(_sync_ptr->flags)))
-		return -EFAULT;
-	if (copy_from_user(&sync_ptr.c.control, &(_sync_ptr->c.control), sizeof(struct snd_pcm_mmap_control)))
-		return -EFAULT;	
 	status = runtime->status;
 	control = runtime->control;
-	if (sync_ptr.flags & SNDRV_PCM_SYNC_PTR_HWSYNC) {
+	if (sync_ptr->flags & SNDRV_PCM_SYNC_PTR_HWSYNC) {
 		err = snd_pcm_hwsync(substream);
 		if (err < 0)
 			return err;
 	}
 	snd_pcm_stream_lock_irq(substream);
-	if (!(sync_ptr.flags & SNDRV_PCM_SYNC_PTR_APPL)) {
+	if (!(sync_ptr->flags & SNDRV_PCM_SYNC_PTR_APPL)) {
 		err = pcm_lib_apply_appl_ptr(substream,
-					     sync_ptr.c.control.appl_ptr);
+					     sync_ptr->c.control.appl_ptr);
 		if (err < 0) {
 			snd_pcm_stream_unlock_irq(substream);
 			return err;
 		}
 	} else {
-		sync_ptr.c.control.appl_ptr = control->appl_ptr;
+		sync_ptr->c.control.appl_ptr = control->appl_ptr;
 	}
-	if (!(sync_ptr.flags & SNDRV_PCM_SYNC_PTR_AVAIL_MIN))
-		control->avail_min = sync_ptr.c.control.avail_min;
+	if (!(sync_ptr->flags & SNDRV_PCM_SYNC_PTR_AVAIL_MIN))
+		control->avail_min = sync_ptr->c.control.avail_min;
 	else
-		sync_ptr.c.control.avail_min = control->avail_min;
-	sync_ptr.s.status.state = status->state;
-	sync_ptr.s.status.hw_ptr = status->hw_ptr;
-	sync_ptr.s.status.tstamp = status->tstamp;
-	sync_ptr.s.status.suspended_state = status->suspended_state;
+		sync_ptr->c.control.avail_min = control->avail_min;
+	sync_ptr->s.status.state = status->state;
+	sync_ptr->s.status.hw_ptr = status->hw_ptr;
+	sync_ptr->s.status.tstamp_sec = status->tstamp_sec;
+	sync_ptr->s.status.tstamp_nsec = status->tstamp_nsec;
+	sync_ptr->s.status.suspended_state = status->suspended_state;
 	snd_pcm_stream_unlock_irq(substream);
+
+	return 0;
+}
+
+#if __BITS_PER_LONG == 32
+static int snd_pcm_sync_ptr32(struct snd_pcm_substream *substream,
+			      struct snd_pcm_sync_ptr32 __user *_sync_ptr)
+{
+	struct snd_pcm_sync_ptr64 sync_ptr64;
+	struct snd_pcm_sync_ptr32 sync_ptr32;
+	int ret;
+
+	memset(&sync_ptr64, 0, sizeof(sync_ptr64));
+	memset(&sync_ptr32, 0, sizeof(sync_ptr32));
+	if (get_user(sync_ptr64.flags, (unsigned __user *)&(_sync_ptr->flags)))
+		return -EFAULT;
+	if (copy_from_user(&sync_ptr64.c.control, &(_sync_ptr->c.control),
+			   sizeof(struct snd_pcm_mmap_control)))
+		return -EFAULT;
+
+	ret = snd_pcm_sync_ptr(substream, &sync_ptr64);
+	if (ret)
+		return ret;
+
+	sync_ptr32.s.status = (struct snd_pcm_mmap_status32) {
+		.state = sync_ptr64.s.status.state,
+		.hw_ptr = sync_ptr64.s.status.hw_ptr,
+		.tstamp_sec = sync_ptr64.s.status.tstamp_sec,
+		.tstamp_nsec = sync_ptr64.s.status.tstamp_nsec,
+		.suspended_state = sync_ptr64.s.status.suspended_state,
+		.audio_tstamp_sec = sync_ptr64.s.status.audio_tstamp_sec,
+		.audio_tstamp_nsec = sync_ptr64.s.status.audio_tstamp_nsec,
+	};
+
+	sync_ptr32.c.control = (struct snd_pcm_mmap_control) {
+		.appl_ptr = sync_ptr64.c.control.appl_ptr,
+		.avail_min = sync_ptr64.c.control.avail_min,
+	};
+
+	if (copy_to_user(_sync_ptr, &sync_ptr32, sizeof(sync_ptr32)))
+		return -EFAULT;
+
+	return 0;
+}
+#endif
+
+static int snd_pcm_sync_ptr64(struct snd_pcm_substream *substream,
+			      struct snd_pcm_sync_ptr64 __user *_sync_ptr)
+{
+	struct snd_pcm_sync_ptr64 sync_ptr;
+	int ret;
+
+	memset(&sync_ptr, 0, sizeof(sync_ptr));
+	if (get_user(sync_ptr.flags, (unsigned __user *)&(_sync_ptr->flags)))
+		return -EFAULT;
+	if (copy_from_user(&sync_ptr.c.control, &(_sync_ptr->c.control),
+			   sizeof(struct snd_pcm_mmap_control)))
+		return -EFAULT;
+
+	ret = snd_pcm_sync_ptr(substream, &sync_ptr);
+	if (ret)
+		return ret;
+
 	if (copy_to_user(_sync_ptr, &sync_ptr, sizeof(sync_ptr)))
 		return -EFAULT;
+
 	return 0;
 }
 
@@ -2987,8 +3046,17 @@ static int snd_pcm_common_ioctl(struct file *file,
 			return -EFAULT;
 		return 0;
 	}
-	case SNDRV_PCM_IOCTL_SYNC_PTR:
-		return snd_pcm_sync_ptr(substream, arg);
+	case SNDRV_PCM_IOCTL_SYNC_PTR64:
+	{
+#if __BITS_PER_LONG == 64
+		return snd_pcm_sync_ptr64(substream, arg);
+#else
+		if (sizeof(__kernel_long_t) == sizeof(time_t))
+			return snd_pcm_sync_ptr32(substream, arg);
+		else
+			return snd_pcm_sync_ptr64(substream, arg);
+#endif
+	}
 #ifdef CONFIG_SND_SUPPORT_OLD_API
 	case SNDRV_PCM_IOCTL_HW_REFINE_OLD:
 		return snd_pcm_hw_refine_old_user(substream, arg);
@@ -3321,7 +3389,7 @@ static int snd_pcm_mmap_status(struct snd_pcm_substream *substream, struct file
 	if (!(area->vm_flags & VM_READ))
 		return -EINVAL;
 	size = area->vm_end - area->vm_start;
-	if (size != PAGE_ALIGN(sizeof(struct snd_pcm_mmap_status)))
+	if (size != PAGE_ALIGN(sizeof(struct snd_pcm_mmap_status64)))
 		return -EINVAL;
 	area->vm_ops = &snd_pcm_vm_ops_status;
 	area->vm_private_data = substream;
-- 
1.7.9.5

[RFC PATCH v2 7/7] sound: core: Avoid using timespec for struct snd_timer_tread

[Baolin Wang]

The struct snd_timer_tread will use 'timespec' type variables to record
timestamp, which is not year 2038 safe on 32bits system.

Since the struct snd_timer_tread is passed through read() rather than
ioctl(), and the read syscall has no command number that lets us pick
between the 32-bit or 64-bit version of this structure.

Thus we introduced one new command SNDRV_TIMER_IOCTL_TREAD64 and new
struct snd_timer_tread64 replacing timespec with s64 type to handle
64bit time_t. Also add one struct compat_snd_timer_tread64_x86_32 to
handle 64bit time_t with 32bit alignment. That means we will set
tu->tread = TREAD_FORMAT_64BIT when user space has a 64bit time_t,
then we will copy to user with struct snd_timer_tread64. For x86_32
mode, we will set tu->tread = TREAD_FORMAT_TIME32_X86 to copy
struct compat_snd_timer_tread64_x86_32 for user. Otherwise we will
use 32bit time_t variables when copying to user.

Moreover this patch replaces timespec type with timespec64 type and
related y2038 safe APIs.

Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
 include/uapi/sound/asound.h |   11 ++-
 sound/core/timer.c          |  173 ++++++++++++++++++++++++++++++++++---------
 sound/core/timer_compat.c   |   10 ++-
 3 files changed, 157 insertions(+), 37 deletions(-)

diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
index fabb283..4c74f52 100644
--- a/include/uapi/sound/asound.h
+++ b/include/uapi/sound/asound.h
@@ -760,7 +760,7 @@ struct snd_timer_status {
 
 #define SNDRV_TIMER_IOCTL_PVERSION	_IOR('T', 0x00, int)
 #define SNDRV_TIMER_IOCTL_NEXT_DEVICE	_IOWR('T', 0x01, struct snd_timer_id)
-#define SNDRV_TIMER_IOCTL_TREAD		_IOW('T', 0x02, int)
+#define SNDRV_TIMER_IOCTL_TREAD_OLD	_IOW('T', 0x02, int)
 #define SNDRV_TIMER_IOCTL_GINFO		_IOWR('T', 0x03, struct snd_timer_ginfo)
 #define SNDRV_TIMER_IOCTL_GPARAMS	_IOW('T', 0x04, struct snd_timer_gparams)
 #define SNDRV_TIMER_IOCTL_GSTATUS	_IOWR('T', 0x05, struct snd_timer_gstatus)
@@ -773,6 +773,15 @@ struct snd_timer_status {
 #define SNDRV_TIMER_IOCTL_STOP		_IO('T', 0xa1)
 #define SNDRV_TIMER_IOCTL_CONTINUE	_IO('T', 0xa2)
 #define SNDRV_TIMER_IOCTL_PAUSE		_IO('T', 0xa3)
+#define SNDRV_TIMER_IOCTL_TREAD64	_IOW('T', 0xa4, int)
+
+#if __BITS_PER_LONG == 64
+#define SNDRV_TIMER_IOCTL_TREAD SNDRV_TIMER_IOCTL_TREAD_OLD
+#else
+#define SNDRV_TIMER_IOCTL_TREAD ((sizeof(__kernel_long_t) > sizeof(time_t)) ? \
+				 SNDRV_TIMER_IOCTL_TREAD_OLD : \
+				 SNDRV_TIMER_IOCTL_TREAD64)
+#endif
 
 struct snd_timer_read {
 	unsigned int resolution;
diff --git a/sound/core/timer.c b/sound/core/timer.c
index 9168365..ba6c09e 100644
--- a/sound/core/timer.c
+++ b/sound/core/timer.c
@@ -58,6 +58,30 @@
 MODULE_ALIAS_CHARDEV(CONFIG_SND_MAJOR, SNDRV_MINOR_TIMER);
 MODULE_ALIAS("devname:snd/timer");
 
+enum timer_tread_format {
+	TREAD_FORMAT_NONE = 0,
+	TREAD_FORMAT_64BIT,
+	TREAD_FORMAT_TIME64,
+	TREAD_FORMAT_TIME32_X86,
+	TREAD_FORMAT_TIME32,
+};
+
+struct snd_timer_tread64 {
+	int event;
+	u32 pad1;
+	s64 tstamp_sec;
+	s64 tstamp_nsec;
+	unsigned int val;
+	u32 pad2;
+};
+
+struct compat_snd_timer_tread64_x86_32 {
+	int event;
+	s64 tstamp_sec;
+	s64 tstamp_nsec;
+	unsigned int val;
+} __packed;
+
 struct snd_timer_user {
 	struct snd_timer_instance *timeri;
 	int tread;		/* enhanced read with timestamps and events */
@@ -69,7 +93,7 @@ struct snd_timer_user {
 	int queue_size;
 	bool disconnected;
 	struct snd_timer_read *queue;
-	struct snd_timer_tread *tqueue;
+	struct snd_timer_tread64 *tqueue;
 	spinlock_t qlock;
 	unsigned long last_resolution;
 	unsigned int filter;
@@ -1262,7 +1286,7 @@ static void snd_timer_user_interrupt(struct snd_timer_instance *timeri,
 }
 
 static void snd_timer_user_append_to_tqueue(struct snd_timer_user *tu,
-					    struct snd_timer_tread *tread)
+					    struct snd_timer_tread64 *tread)
 {
 	if (tu->qused >= tu->queue_size) {
 		tu->overrun++;
@@ -1279,7 +1303,7 @@ static void snd_timer_user_ccallback(struct snd_timer_instance *timeri,
 				     unsigned long resolution)
 {
 	struct snd_timer_user *tu = timeri->callback_data;
-	struct snd_timer_tread r1;
+	struct snd_timer_tread64 r1;
 	unsigned long flags;
 
 	if (event >= SNDRV_TIMER_EVENT_START &&
@@ -1289,7 +1313,8 @@ static void snd_timer_user_ccallback(struct snd_timer_instance *timeri,
 		return;
 	memset(&r1, 0, sizeof(r1));
 	r1.event = event;
-	r1.tstamp = timespec64_to_timespec(*tstamp);
+	r1.tstamp_sec = tstamp->tv_sec;
+	r1.tstamp_nsec = tstamp->tv_nsec;
 	r1.val = resolution;
 	spin_lock_irqsave(&tu->qlock, flags);
 	snd_timer_user_append_to_tqueue(tu, &r1);
@@ -1311,7 +1336,7 @@ static void snd_timer_user_tinterrupt(struct snd_timer_instance *timeri,
 				      unsigned long ticks)
 {
 	struct snd_timer_user *tu = timeri->callback_data;
-	struct snd_timer_tread *r, r1;
+	struct snd_timer_tread64 *r, r1;
 	struct timespec64 tstamp;
 	int prev, append = 0;
 
@@ -1332,7 +1357,8 @@ static void snd_timer_user_tinterrupt(struct snd_timer_instance *timeri,
 	if ((tu->filter & (1 << SNDRV_TIMER_EVENT_RESOLUTION)) &&
 	    tu->last_resolution != resolution) {
 		r1.event = SNDRV_TIMER_EVENT_RESOLUTION;
-		r1.tstamp = timespec64_to_timespec(tstamp);
+		r1.tstamp_sec = tstamp.tv_sec;
+		r1.tstamp_nsec = tstamp.tv_nsec;
 		r1.val = resolution;
 		snd_timer_user_append_to_tqueue(tu, &r1);
 		tu->last_resolution = resolution;
@@ -1346,14 +1372,16 @@ static void snd_timer_user_tinterrupt(struct snd_timer_instance *timeri,
 		prev = tu->qtail == 0 ? tu->queue_size - 1 : tu->qtail - 1;
 		r = &tu->tqueue[prev];
 		if (r->event == SNDRV_TIMER_EVENT_TICK) {
-			r->tstamp = timespec64_to_timespec(tstamp);
+			r->tstamp_sec = tstamp.tv_sec;
+			r->tstamp_nsec = tstamp.tv_nsec;
 			r->val += ticks;
 			append++;
 			goto __wake;
 		}
 	}
 	r1.event = SNDRV_TIMER_EVENT_TICK;
-	r1.tstamp = timespec64_to_timespec(tstamp);
+	r1.tstamp_sec = tstamp.tv_sec;
+	r1.tstamp_nsec = tstamp.tv_nsec;
 	r1.val = ticks;
 	snd_timer_user_append_to_tqueue(tu, &r1);
 	append++;
@@ -1368,7 +1396,7 @@ static void snd_timer_user_tinterrupt(struct snd_timer_instance *timeri,
 static int realloc_user_queue(struct snd_timer_user *tu, int size)
 {
 	struct snd_timer_read *queue = NULL;
-	struct snd_timer_tread *tqueue = NULL;
+	struct snd_timer_tread64 *tqueue = NULL;
 
 	if (tu->tread) {
 		tqueue = kcalloc(size, sizeof(*tqueue), GFP_KERNEL);
@@ -1797,11 +1825,11 @@ static int snd_timer_user_params(struct file *file,
 	tu->qhead = tu->qtail = tu->qused = 0;
 	if (tu->timeri->flags & SNDRV_TIMER_IFLG_EARLY_EVENT) {
 		if (tu->tread) {
-			struct snd_timer_tread tread;
+			struct snd_timer_tread64 tread;
 			memset(&tread, 0, sizeof(tread));
 			tread.event = SNDRV_TIMER_EVENT_EARLY;
-			tread.tstamp.tv_sec = 0;
-			tread.tstamp.tv_nsec = 0;
+			tread.tstamp_sec = 0;
+			tread.tstamp_nsec = 0;
 			tread.val = 0;
 			snd_timer_user_append_to_tqueue(tu, &tread);
 		} else {
@@ -1919,6 +1947,39 @@ static int snd_timer_user_pause(struct file *file)
 	return (err = snd_timer_pause(tu->timeri)) < 0 ? err : 0;
 }
 
+static int snd_timer_user_tread(void __user *argp, struct snd_timer_user *tu,
+				unsigned int cmd, bool compat)
+{
+	int __user *p = argp;
+	int xarg, old_tread;
+
+	if (tu->timeri)	/* too late */
+		return -EBUSY;
+	if (get_user(xarg, p))
+		return -EFAULT;
+
+	old_tread = tu->tread;
+
+	if (!xarg)
+		tu->tread = TREAD_FORMAT_NONE;
+	else if (!IS_ENABLED(CONFIG_64BITS) && !compat)
+		tu->tread = TREAD_FORMAT_64BIT;
+	else if (cmd == SNDRV_TIMER_IOCTL_TREAD64)
+		tu->tread = TREAD_FORMAT_TIME64;
+	else if (IS_ENABLED(CONFIG_X86))
+		tu->tread = TREAD_FORMAT_TIME32_X86;
+	else
+		tu->tread = TREAD_FORMAT_TIME32;
+
+	if (tu->tread != old_tread &&
+	    realloc_user_queue(tu, tu->queue_size) < 0) {
+		tu->tread = old_tread;
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+
 enum {
 	SNDRV_TIMER_IOCTL_START_OLD = _IO('T', 0x20),
 	SNDRV_TIMER_IOCTL_STOP_OLD = _IO('T', 0x21),
@@ -1927,7 +1988,7 @@ enum {
 };
 
 static long __snd_timer_user_ioctl(struct file *file, unsigned int cmd,
-				 unsigned long arg)
+				 unsigned long arg, bool compat)
 {
 	struct snd_timer_user *tu;
 	void __user *argp = (void __user *)arg;
@@ -1939,23 +2000,9 @@ static long __snd_timer_user_ioctl(struct file *file, unsigned int cmd,
 		return put_user(SNDRV_TIMER_VERSION, p) ? -EFAULT : 0;
 	case SNDRV_TIMER_IOCTL_NEXT_DEVICE:
 		return snd_timer_user_next_device(argp);
-	case SNDRV_TIMER_IOCTL_TREAD:
-	{
-		int xarg, old_tread;
-
-		if (tu->timeri)	/* too late */
-			return -EBUSY;
-		if (get_user(xarg, p))
-			return -EFAULT;
-		old_tread = tu->tread;
-		tu->tread = xarg ? 1 : 0;
-		if (tu->tread != old_tread &&
-		    realloc_user_queue(tu, tu->queue_size) < 0) {
-			tu->tread = old_tread;
-			return -ENOMEM;
-		}
-		return 0;
-	}
+	case SNDRV_TIMER_IOCTL_TREAD_OLD:
+	case SNDRV_TIMER_IOCTL_TREAD64:
+		return snd_timer_user_tread(argp, tu, cmd, compat);
 	case SNDRV_TIMER_IOCTL_GINFO:
 		return snd_timer_user_ginfo(file, argp);
 	case SNDRV_TIMER_IOCTL_GPARAMS:
@@ -1995,7 +2042,7 @@ static long snd_timer_user_ioctl(struct file *file, unsigned int cmd,
 	long ret;
 
 	mutex_lock(&tu->ioctl_lock);
-	ret = __snd_timer_user_ioctl(file, cmd, arg);
+	ret = __snd_timer_user_ioctl(file, cmd, arg, false);
 	mutex_unlock(&tu->ioctl_lock);
 	return ret;
 }
@@ -2017,7 +2064,24 @@ static ssize_t snd_timer_user_read(struct file *file, char __user *buffer,
 	int err = 0;
 
 	tu = file->private_data;
-	unit = tu->tread ? sizeof(struct snd_timer_tread) : sizeof(struct snd_timer_read);
+	switch (tu->tread) {
+	case TREAD_FORMAT_TIME32_X86:
+		unit = sizeof(struct compat_snd_timer_tread64_x86_32);
+		break;
+	case TREAD_FORMAT_64BIT:
+	case TREAD_FORMAT_TIME64:
+		unit = sizeof(struct snd_timer_tread64);
+		break;
+	case TREAD_FORMAT_TIME32:
+		unit = sizeof(struct snd_timer_tread);
+		break;
+	case TREAD_FORMAT_NONE:
+		unit = sizeof(struct snd_timer_read);
+		break;
+	default:
+		return -ENOTSUPP;
+	}
+
 	mutex_lock(&tu->ioctl_lock);
 	spin_lock_irq(&tu->qlock);
 	while ((long)count - result >= unit) {
@@ -2057,9 +2121,48 @@ static ssize_t snd_timer_user_read(struct file *file, char __user *buffer,
 		spin_unlock_irq(&tu->qlock);
 
 		if (tu->tread) {
-			if (copy_to_user(buffer, &tu->tqueue[qhead],
-					 sizeof(struct snd_timer_tread)))
-				err = -EFAULT;
+			struct snd_timer_tread64 *tread = &tu->tqueue[qhead];
+			struct snd_timer_tread tread32;
+			struct compat_snd_timer_tread64_x86_32 compat_tread64;
+
+			switch (tu->tread) {
+			case TREAD_FORMAT_TIME32_X86:
+				memset(&compat_tread64, 0, sizeof(compat_tread64));
+				compat_tread64 = (struct compat_snd_timer_tread64_x86_32) {
+					.event = tread->event,
+					.tstamp_sec = tread->tstamp_sec,
+					.tstamp_nsec = tread->tstamp_nsec,
+					.val = tread->val,
+				};
+
+				if (copy_to_user(buffer, &compat_tread64,
+						 sizeof(compat_tread64)))
+					err = -EFAULT;
+				break;
+			case TREAD_FORMAT_64BIT:
+			case TREAD_FORMAT_TIME64:
+				if (copy_to_user(buffer, tread,
+						 sizeof(struct snd_timer_tread64)))
+					err = -EFAULT;
+				break;
+			case TREAD_FORMAT_TIME32:
+				memset(&tread32, 0, sizeof(tread32));
+				tread32 = (struct snd_timer_tread) {
+					.event = tread->event,
+					.tstamp = {
+						.tv_sec = tread->tstamp_sec,
+						.tv_nsec = tread->tstamp_nsec,
+					},
+					.val = tread->val,
+				};
+
+				if (copy_to_user(buffer, &tread32, sizeof(tread32)))
+					err = -EFAULT;
+				break;
+			default:
+				err = -ENOTSUPP;
+				break;
+			}
 		} else {
 			if (copy_to_user(buffer, &tu->queue[qhead],
 					 sizeof(struct snd_timer_read)))
diff --git a/sound/core/timer_compat.c b/sound/core/timer_compat.c
index 3e09548..866e090 100644
--- a/sound/core/timer_compat.c
+++ b/sound/core/timer_compat.c
@@ -110,7 +110,15 @@ static long snd_timer_user_ioctl_compat(struct file *file, unsigned int cmd, uns
 	case SNDRV_TIMER_IOCTL_PAUSE:
 	case SNDRV_TIMER_IOCTL_PAUSE_OLD:
 	case SNDRV_TIMER_IOCTL_NEXT_DEVICE:
-		return snd_timer_user_ioctl(file, cmd, (unsigned long)argp);
+	{
+		struct snd_timer_user *tu = file->private_data;
+		long ret;
+
+		mutex_lock(&tu->ioctl_lock);
+		ret = __snd_timer_user_ioctl(file, cmd, arg, false);
+		mutex_unlock(&tu->ioctl_lock);
+		return ret;
+	}
 	case SNDRV_TIMER_IOCTL_GPARAMS32:
 		return snd_timer_user_gparams_compat(file, argp);
 	case SNDRV_TIMER_IOCTL_INFO32:
-- 
1.7.9.5

Re: [alsa-devel] [RFC PATCH v2 7/7] sound: core: Avoid using timespec for struct snd_timer_tread

[Takashi Iwai]

On Thu, 02 Nov 2017 12:06:57 +0100,
Baolin Wang wrote:
> 
> The struct snd_timer_tread will use 'timespec' type variables to record
> timestamp, which is not year 2038 safe on 32bits system.
> 
> Since the struct snd_timer_tread is passed through read() rather than
> ioctl(), and the read syscall has no command number that lets us pick
> between the 32-bit or 64-bit version of this structure.
> 
> Thus we introduced one new command SNDRV_TIMER_IOCTL_TREAD64 and new
> struct snd_timer_tread64 replacing timespec with s64 type to handle
> 64bit time_t. Also add one struct compat_snd_timer_tread64_x86_32 to
> handle 64bit time_t with 32bit alignment. That means we will set
> tu->tread = TREAD_FORMAT_64BIT when user space has a 64bit time_t,
> then we will copy to user with struct snd_timer_tread64. For x86_32
> mode, we will set tu->tread = TREAD_FORMAT_TIME32_X86 to copy
> struct compat_snd_timer_tread64_x86_32 for user. Otherwise we will
> use 32bit time_t variables when copying to user.

We should introduce SNDRV_TIMER_IOCTL_USER_PVERSION instead, where
user-space can tell which protocol version it understands.  If the
protocol version is higher than some definition, we can assume it's
64-bit ready.  The *_USER_PVERSION is issued from alsa-lib side.
In that way, we can extend the ABI more flexibly.  A similar trick was
already used in PCM ABI.  (Ditto for control and rawmidi API; we
should have the same mechanism for all relevant APIs).

Moreover, once when the new protocol is used, we can use the standard
64bit monotonic nsecs as a timestamp, so that we don't need to care
about 32/64bit compatibility.


thanks,

Takashi

> Moreover this patch replaces timespec type with timespec64 type and
> related y2038 safe APIs.
> 
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---
>  include/uapi/sound/asound.h |   11 ++-
>  sound/core/timer.c          |  173 ++++++++++++++++++++++++++++++++++---------
>  sound/core/timer_compat.c   |   10 ++-
>  3 files changed, 157 insertions(+), 37 deletions(-)
> 
> diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
> index fabb283..4c74f52 100644
> --- a/include/uapi/sound/asound.h
> +++ b/include/uapi/sound/asound.h
> @@ -760,7 +760,7 @@ struct snd_timer_status {
>  
>  #define SNDRV_TIMER_IOCTL_PVERSION	_IOR('T', 0x00, int)
>  #define SNDRV_TIMER_IOCTL_NEXT_DEVICE	_IOWR('T', 0x01, struct snd_timer_id)
> -#define SNDRV_TIMER_IOCTL_TREAD		_IOW('T', 0x02, int)
> +#define SNDRV_TIMER_IOCTL_TREAD_OLD	_IOW('T', 0x02, int)
>  #define SNDRV_TIMER_IOCTL_GINFO		_IOWR('T', 0x03, struct snd_timer_ginfo)
>  #define SNDRV_TIMER_IOCTL_GPARAMS	_IOW('T', 0x04, struct snd_timer_gparams)
>  #define SNDRV_TIMER_IOCTL_GSTATUS	_IOWR('T', 0x05, struct snd_timer_gstatus)
> @@ -773,6 +773,15 @@ struct snd_timer_status {
>  #define SNDRV_TIMER_IOCTL_STOP		_IO('T', 0xa1)
>  #define SNDRV_TIMER_IOCTL_CONTINUE	_IO('T', 0xa2)
>  #define SNDRV_TIMER_IOCTL_PAUSE		_IO('T', 0xa3)
> +#define SNDRV_TIMER_IOCTL_TREAD64	_IOW('T', 0xa4, int)
> +
> +#if __BITS_PER_LONG == 64
> +#define SNDRV_TIMER_IOCTL_TREAD SNDRV_TIMER_IOCTL_TREAD_OLD
> +#else
> +#define SNDRV_TIMER_IOCTL_TREAD ((sizeof(__kernel_long_t) > sizeof(time_t)) ? \
> +				 SNDRV_TIMER_IOCTL_TREAD_OLD : \
> +				 SNDRV_TIMER_IOCTL_TREAD64)
> +#endif
>  
>  struct snd_timer_read {
>  	unsigned int resolution;
> diff --git a/sound/core/timer.c b/sound/core/timer.c
> index 9168365..ba6c09e 100644
> --- a/sound/core/timer.c
> +++ b/sound/core/timer.c
> @@ -58,6 +58,30 @@
>  MODULE_ALIAS_CHARDEV(CONFIG_SND_MAJOR, SNDRV_MINOR_TIMER);
>  MODULE_ALIAS("devname:snd/timer");
>  
> +enum timer_tread_format {
> +	TREAD_FORMAT_NONE = 0,
> +	TREAD_FORMAT_64BIT,
> +	TREAD_FORMAT_TIME64,
> +	TREAD_FORMAT_TIME32_X86,
> +	TREAD_FORMAT_TIME32,
> +};
> +
> +struct snd_timer_tread64 {
> +	int event;
> +	u32 pad1;
> +	s64 tstamp_sec;
> +	s64 tstamp_nsec;
> +	unsigned int val;
> +	u32 pad2;
> +};
> +
> +struct compat_snd_timer_tread64_x86_32 {
> +	int event;
> +	s64 tstamp_sec;
> +	s64 tstamp_nsec;
> +	unsigned int val;
> +} __packed;
> +
>  struct snd_timer_user {
>  	struct snd_timer_instance *timeri;
>  	int tread;		/* enhanced read with timestamps and events */
> @@ -69,7 +93,7 @@ struct snd_timer_user {
>  	int queue_size;
>  	bool disconnected;
>  	struct snd_timer_read *queue;
> -	struct snd_timer_tread *tqueue;
> +	struct snd_timer_tread64 *tqueue;
>  	spinlock_t qlock;
>  	unsigned long last_resolution;
>  	unsigned int filter;
> @@ -1262,7 +1286,7 @@ static void snd_timer_user_interrupt(struct snd_timer_instance *timeri,
>  }
>  
>  static void snd_timer_user_append_to_tqueue(struct snd_timer_user *tu,
> -					    struct snd_timer_tread *tread)
> +					    struct snd_timer_tread64 *tread)
>  {
>  	if (tu->qused >= tu->queue_size) {
>  		tu->overrun++;
> @@ -1279,7 +1303,7 @@ static void snd_timer_user_ccallback(struct snd_timer_instance *timeri,
>  				     unsigned long resolution)
>  {
>  	struct snd_timer_user *tu = timeri->callback_data;
> -	struct snd_timer_tread r1;
> +	struct snd_timer_tread64 r1;
>  	unsigned long flags;
>  
>  	if (event >= SNDRV_TIMER_EVENT_START &&
> @@ -1289,7 +1313,8 @@ static void snd_timer_user_ccallback(struct snd_timer_instance *timeri,
>  		return;
>  	memset(&r1, 0, sizeof(r1));
>  	r1.event = event;
> -	r1.tstamp = timespec64_to_timespec(*tstamp);
> +	r1.tstamp_sec = tstamp->tv_sec;
> +	r1.tstamp_nsec = tstamp->tv_nsec;
>  	r1.val = resolution;
>  	spin_lock_irqsave(&tu->qlock, flags);
>  	snd_timer_user_append_to_tqueue(tu, &r1);
> @@ -1311,7 +1336,7 @@ static void snd_timer_user_tinterrupt(struct snd_timer_instance *timeri,
>  				      unsigned long ticks)
>  {
>  	struct snd_timer_user *tu = timeri->callback_data;
> -	struct snd_timer_tread *r, r1;
> +	struct snd_timer_tread64 *r, r1;
>  	struct timespec64 tstamp;
>  	int prev, append = 0;
>  
> @@ -1332,7 +1357,8 @@ static void snd_timer_user_tinterrupt(struct snd_timer_instance *timeri,
>  	if ((tu->filter & (1 << SNDRV_TIMER_EVENT_RESOLUTION)) &&
>  	    tu->last_resolution != resolution) {
>  		r1.event = SNDRV_TIMER_EVENT_RESOLUTION;
> -		r1.tstamp = timespec64_to_timespec(tstamp);
> +		r1.tstamp_sec = tstamp.tv_sec;
> +		r1.tstamp_nsec = tstamp.tv_nsec;
>  		r1.val = resolution;
>  		snd_timer_user_append_to_tqueue(tu, &r1);
>  		tu->last_resolution = resolution;
> @@ -1346,14 +1372,16 @@ static void snd_timer_user_tinterrupt(struct snd_timer_instance *timeri,
>  		prev = tu->qtail == 0 ? tu->queue_size - 1 : tu->qtail - 1;
>  		r = &tu->tqueue[prev];
>  		if (r->event == SNDRV_TIMER_EVENT_TICK) {
> -			r->tstamp = timespec64_to_timespec(tstamp);
> +			r->tstamp_sec = tstamp.tv_sec;
> +			r->tstamp_nsec = tstamp.tv_nsec;
>  			r->val += ticks;
>  			append++;
>  			goto __wake;
>  		}
>  	}
>  	r1.event = SNDRV_TIMER_EVENT_TICK;
> -	r1.tstamp = timespec64_to_timespec(tstamp);
> +	r1.tstamp_sec = tstamp.tv_sec;
> +	r1.tstamp_nsec = tstamp.tv_nsec;
>  	r1.val = ticks;
>  	snd_timer_user_append_to_tqueue(tu, &r1);
>  	append++;
> @@ -1368,7 +1396,7 @@ static void snd_timer_user_tinterrupt(struct snd_timer_instance *timeri,
>  static int realloc_user_queue(struct snd_timer_user *tu, int size)
>  {
>  	struct snd_timer_read *queue = NULL;
> -	struct snd_timer_tread *tqueue = NULL;
> +	struct snd_timer_tread64 *tqueue = NULL;
>  
>  	if (tu->tread) {
>  		tqueue = kcalloc(size, sizeof(*tqueue), GFP_KERNEL);
> @@ -1797,11 +1825,11 @@ static int snd_timer_user_params(struct file *file,
>  	tu->qhead = tu->qtail = tu->qused = 0;
>  	if (tu->timeri->flags & SNDRV_TIMER_IFLG_EARLY_EVENT) {
>  		if (tu->tread) {
> -			struct snd_timer_tread tread;
> +			struct snd_timer_tread64 tread;
>  			memset(&tread, 0, sizeof(tread));
>  			tread.event = SNDRV_TIMER_EVENT_EARLY;
> -			tread.tstamp.tv_sec = 0;
> -			tread.tstamp.tv_nsec = 0;
> +			tread.tstamp_sec = 0;
> +			tread.tstamp_nsec = 0;
>  			tread.val = 0;
>  			snd_timer_user_append_to_tqueue(tu, &tread);
>  		} else {
> @@ -1919,6 +1947,39 @@ static int snd_timer_user_pause(struct file *file)
>  	return (err = snd_timer_pause(tu->timeri)) < 0 ? err : 0;
>  }
>  
> +static int snd_timer_user_tread(void __user *argp, struct snd_timer_user *tu,
> +				unsigned int cmd, bool compat)
> +{
> +	int __user *p = argp;
> +	int xarg, old_tread;
> +
> +	if (tu->timeri)	/* too late */
> +		return -EBUSY;
> +	if (get_user(xarg, p))
> +		return -EFAULT;
> +
> +	old_tread = tu->tread;
> +
> +	if (!xarg)
> +		tu->tread = TREAD_FORMAT_NONE;
> +	else if (!IS_ENABLED(CONFIG_64BITS) && !compat)
> +		tu->tread = TREAD_FORMAT_64BIT;
> +	else if (cmd == SNDRV_TIMER_IOCTL_TREAD64)
> +		tu->tread = TREAD_FORMAT_TIME64;
> +	else if (IS_ENABLED(CONFIG_X86))
> +		tu->tread = TREAD_FORMAT_TIME32_X86;
> +	else
> +		tu->tread = TREAD_FORMAT_TIME32;
> +
> +	if (tu->tread != old_tread &&
> +	    realloc_user_queue(tu, tu->queue_size) < 0) {
> +		tu->tread = old_tread;
> +		return -ENOMEM;
> +	}
> +
> +	return 0;
> +}
> +
>  enum {
>  	SNDRV_TIMER_IOCTL_START_OLD = _IO('T', 0x20),
>  	SNDRV_TIMER_IOCTL_STOP_OLD = _IO('T', 0x21),
> @@ -1927,7 +1988,7 @@ enum {
>  };
>  
>  static long __snd_timer_user_ioctl(struct file *file, unsigned int cmd,
> -				 unsigned long arg)
> +				 unsigned long arg, bool compat)
>  {
>  	struct snd_timer_user *tu;
>  	void __user *argp = (void __user *)arg;
> @@ -1939,23 +2000,9 @@ static long __snd_timer_user_ioctl(struct file *file, unsigned int cmd,
>  		return put_user(SNDRV_TIMER_VERSION, p) ? -EFAULT : 0;
>  	case SNDRV_TIMER_IOCTL_NEXT_DEVICE:
>  		return snd_timer_user_next_device(argp);
> -	case SNDRV_TIMER_IOCTL_TREAD:
> -	{
> -		int xarg, old_tread;
> -
> -		if (tu->timeri)	/* too late */
> -			return -EBUSY;
> -		if (get_user(xarg, p))
> -			return -EFAULT;
> -		old_tread = tu->tread;
> -		tu->tread = xarg ? 1 : 0;
> -		if (tu->tread != old_tread &&
> -		    realloc_user_queue(tu, tu->queue_size) < 0) {
> -			tu->tread = old_tread;
> -			return -ENOMEM;
> -		}
> -		return 0;
> -	}
> +	case SNDRV_TIMER_IOCTL_TREAD_OLD:
> +	case SNDRV_TIMER_IOCTL_TREAD64:
> +		return snd_timer_user_tread(argp, tu, cmd, compat);
>  	case SNDRV_TIMER_IOCTL_GINFO:
>  		return snd_timer_user_ginfo(file, argp);
>  	case SNDRV_TIMER_IOCTL_GPARAMS:
> @@ -1995,7 +2042,7 @@ static long snd_timer_user_ioctl(struct file *file, unsigned int cmd,
>  	long ret;
>  
>  	mutex_lock(&tu->ioctl_lock);
> -	ret = __snd_timer_user_ioctl(file, cmd, arg);
> +	ret = __snd_timer_user_ioctl(file, cmd, arg, false);
>  	mutex_unlock(&tu->ioctl_lock);
>  	return ret;
>  }
> @@ -2017,7 +2064,24 @@ static ssize_t snd_timer_user_read(struct file *file, char __user *buffer,
>  	int err = 0;
>  
>  	tu = file->private_data;
> -	unit = tu->tread ? sizeof(struct snd_timer_tread) : sizeof(struct snd_timer_read);
> +	switch (tu->tread) {
> +	case TREAD_FORMAT_TIME32_X86:
> +		unit = sizeof(struct compat_snd_timer_tread64_x86_32);
> +		break;
> +	case TREAD_FORMAT_64BIT:
> +	case TREAD_FORMAT_TIME64:
> +		unit = sizeof(struct snd_timer_tread64);
> +		break;
> +	case TREAD_FORMAT_TIME32:
> +		unit = sizeof(struct snd_timer_tread);
> +		break;
> +	case TREAD_FORMAT_NONE:
> +		unit = sizeof(struct snd_timer_read);
> +		break;
> +	default:
> +		return -ENOTSUPP;
> +	}
> +
>  	mutex_lock(&tu->ioctl_lock);
>  	spin_lock_irq(&tu->qlock);
>  	while ((long)count - result >= unit) {
> @@ -2057,9 +2121,48 @@ static ssize_t snd_timer_user_read(struct file *file, char __user *buffer,
>  		spin_unlock_irq(&tu->qlock);
>  
>  		if (tu->tread) {
> -			if (copy_to_user(buffer, &tu->tqueue[qhead],
> -					 sizeof(struct snd_timer_tread)))
> -				err = -EFAULT;
> +			struct snd_timer_tread64 *tread = &tu->tqueue[qhead];
> +			struct snd_timer_tread tread32;
> +			struct compat_snd_timer_tread64_x86_32 compat_tread64;
> +
> +			switch (tu->tread) {
> +			case TREAD_FORMAT_TIME32_X86:
> +				memset(&compat_tread64, 0, sizeof(compat_tread64));
> +				compat_tread64 = (struct compat_snd_timer_tread64_x86_32) {
> +					.event = tread->event,
> +					.tstamp_sec = tread->tstamp_sec,
> +					.tstamp_nsec = tread->tstamp_nsec,
> +					.val = tread->val,
> +				};
> +
> +				if (copy_to_user(buffer, &compat_tread64,
> +						 sizeof(compat_tread64)))
> +					err = -EFAULT;
> +				break;
> +			case TREAD_FORMAT_64BIT:
> +			case TREAD_FORMAT_TIME64:
> +				if (copy_to_user(buffer, tread,
> +						 sizeof(struct snd_timer_tread64)))
> +					err = -EFAULT;
> +				break;
> +			case TREAD_FORMAT_TIME32:
> +				memset(&tread32, 0, sizeof(tread32));
> +				tread32 = (struct snd_timer_tread) {
> +					.event = tread->event,
> +					.tstamp = {
> +						.tv_sec = tread->tstamp_sec,
> +						.tv_nsec = tread->tstamp_nsec,
> +					},
> +					.val = tread->val,
> +				};
> +
> +				if (copy_to_user(buffer, &tread32, sizeof(tread32)))
> +					err = -EFAULT;
> +				break;
> +			default:
> +				err = -ENOTSUPP;
> +				break;
> +			}
>  		} else {
>  			if (copy_to_user(buffer, &tu->queue[qhead],
>  					 sizeof(struct snd_timer_read)))
> diff --git a/sound/core/timer_compat.c b/sound/core/timer_compat.c
> index 3e09548..866e090 100644
> --- a/sound/core/timer_compat.c
> +++ b/sound/core/timer_compat.c
> @@ -110,7 +110,15 @@ static long snd_timer_user_ioctl_compat(struct file *file, unsigned int cmd, uns
>  	case SNDRV_TIMER_IOCTL_PAUSE:
>  	case SNDRV_TIMER_IOCTL_PAUSE_OLD:
>  	case SNDRV_TIMER_IOCTL_NEXT_DEVICE:
> -		return snd_timer_user_ioctl(file, cmd, (unsigned long)argp);
> +	{
> +		struct snd_timer_user *tu = file->private_data;
> +		long ret;
> +
> +		mutex_lock(&tu->ioctl_lock);
> +		ret = __snd_timer_user_ioctl(file, cmd, arg, false);
> +		mutex_unlock(&tu->ioctl_lock);
> +		return ret;
> +	}
>  	case SNDRV_TIMER_IOCTL_GPARAMS32:
>  		return snd_timer_user_gparams_compat(file, argp);
>  	case SNDRV_TIMER_IOCTL_INFO32:
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

Re: [alsa-devel] [RFC PATCH v2 7/7] sound: core: Avoid using timespec for struct snd_timer_tread

[Arnd Bergmann]

On Sun, Nov 5, 2017 at 11:29 AM, Takashi Iwai <tiwai@suse.de> wrote:
> On Thu, 02 Nov 2017 12:06:57 +0100,
> Baolin Wang wrote:
>>
>> The struct snd_timer_tread will use 'timespec' type variables to record
>> timestamp, which is not year 2038 safe on 32bits system.
>>
>> Since the struct snd_timer_tread is passed through read() rather than
>> ioctl(), and the read syscall has no command number that lets us pick
>> between the 32-bit or 64-bit version of this structure.
>>
>> Thus we introduced one new command SNDRV_TIMER_IOCTL_TREAD64 and new
>> struct snd_timer_tread64 replacing timespec with s64 type to handle
>> 64bit time_t. Also add one struct compat_snd_timer_tread64_x86_32 to
>> handle 64bit time_t with 32bit alignment. That means we will set
>> tu->tread = TREAD_FORMAT_64BIT when user space has a 64bit time_t,
>> then we will copy to user with struct snd_timer_tread64. For x86_32
>> mode, we will set tu->tread = TREAD_FORMAT_TIME32_X86 to copy
>> struct compat_snd_timer_tread64_x86_32 for user. Otherwise we will
>> use 32bit time_t variables when copying to user.
>
> We should introduce SNDRV_TIMER_IOCTL_USER_PVERSION instead, where
> user-space can tell which protocol version it understands.  If the
> protocol version is higher than some definition, we can assume it's
> 64-bit ready.  The *_USER_PVERSION is issued from alsa-lib side.
> In that way, we can extend the ABI more flexibly.  A similar trick was
> already used in PCM ABI.  (Ditto for control and rawmidi API; we
> should have the same mechanism for all relevant APIs).
>
> Moreover, once when the new protocol is used, we can use the standard
> 64bit monotonic nsecs as a timestamp, so that we don't need to care
> about 32/64bit compatibility.

I think that's fine, we can do that too, but I don't see how we get around
to doing something like Baolin's patch first. Without this, we will get
existing user space code compiling against our kernel headers using a
new glibc release that will inadvertently change the structure layout
on the read file descriptor.

The trick with redefining SNDRV_TIMER_IOCTL_TREAD in that
configuration lets the kernel know what API the user space expects
without requiring source-level changes.

If you want to for all users of SNDRV_TIMER_IOCTL_TREAD to move
to a new interface for y2038-safety, we'd have to redefined the structure
to avoid the libc-provided 'struct timespec' on 32-bit architectures, like:

diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
index 299a822d2c4e..f93cace4cd24 100644
--- a/include/uapi/sound/asound.h
+++ b/include/uapi/sound/asound.h
@@ -801,7 +801,14 @@ enum {

 struct snd_timer_tread {
        int event;
+#if __BITS_PER_LONG == 32
+       struct {
+               __kernel_long_t tv_sec;
+               __kernel_long_t tv_usec;
+       };
+#else
        struct timespec tstamp;
+#endif
        unsigned int val;
 };

This has a somewhat higher risk of breaking existing code (since the type
changes), and it doesn't solve the overflow.

      Arnd

Re: [alsa-devel] [RFC PATCH v2 7/7] sound: core: Avoid using timespec for struct snd_timer_tread

[Takashi Iwai]

On Sun, 05 Nov 2017 14:16:28 +0100,
Arnd Bergmann wrote:
> 
> On Sun, Nov 5, 2017 at 11:29 AM, Takashi Iwai <tiwai@suse.de> wrote:
> > On Thu, 02 Nov 2017 12:06:57 +0100,
> > Baolin Wang wrote:
> >>
> >> The struct snd_timer_tread will use 'timespec' type variables to record
> >> timestamp, which is not year 2038 safe on 32bits system.
> >>
> >> Since the struct snd_timer_tread is passed through read() rather than
> >> ioctl(), and the read syscall has no command number that lets us pick
> >> between the 32-bit or 64-bit version of this structure.
> >>
> >> Thus we introduced one new command SNDRV_TIMER_IOCTL_TREAD64 and new
> >> struct snd_timer_tread64 replacing timespec with s64 type to handle
> >> 64bit time_t. Also add one struct compat_snd_timer_tread64_x86_32 to
> >> handle 64bit time_t with 32bit alignment. That means we will set
> >> tu->tread = TREAD_FORMAT_64BIT when user space has a 64bit time_t,
> >> then we will copy to user with struct snd_timer_tread64. For x86_32
> >> mode, we will set tu->tread = TREAD_FORMAT_TIME32_X86 to copy
> >> struct compat_snd_timer_tread64_x86_32 for user. Otherwise we will
> >> use 32bit time_t variables when copying to user.
> >
> > We should introduce SNDRV_TIMER_IOCTL_USER_PVERSION instead, where
> > user-space can tell which protocol version it understands.  If the
> > protocol version is higher than some definition, we can assume it's
> > 64-bit ready.  The *_USER_PVERSION is issued from alsa-lib side.
> > In that way, we can extend the ABI more flexibly.  A similar trick was
> > already used in PCM ABI.  (Ditto for control and rawmidi API; we
> > should have the same mechanism for all relevant APIs).
> >
> > Moreover, once when the new protocol is used, we can use the standard
> > 64bit monotonic nsecs as a timestamp, so that we don't need to care
> > about 32/64bit compatibility.
> 
> I think that's fine, we can do that too, but I don't see how we get around
> to doing something like Baolin's patch first. Without this, we will get
> existing user space code compiling against our kernel headers using a
> new glibc release that will inadvertently change the structure layout
> on the read file descriptor.

But it won't work in anyway in multiple ways, e.g. this timer read
stuff and another the structs embedded in the mmappged page.  If you
do rebuild things with new glibc, it should tell kernel about the new
ABI in anyway more or less explicitly.  And if you need it, it means
that some source-code level API change would be possible.

Of course, passing which data type is another question.  Maybe 64bit
nsecs wouldn't fit all places, and timespec64 style would be still
required.  But still, the current patch looks still too unnecessarily
complex to me.  (Yeah I know that the problem is complex, but the code
can be simpler, I hope!)

> The trick with redefining SNDRV_TIMER_IOCTL_TREAD in that
> configuration lets the kernel know what API the user space expects
> without requiring source-level changes.

Right, it works for this case, but not always.
If jumping the API would give a cleaner way of implementation, I'd
prefer that over too hackeries, IMO.

> If you want to for all users of SNDRV_TIMER_IOCTL_TREAD to move
> to a new interface for y2038-safety, we'd have to redefined the structure
> to avoid the libc-provided 'struct timespec' on 32-bit architectures, like:
> 
> diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
> index 299a822d2c4e..f93cace4cd24 100644
> --- a/include/uapi/sound/asound.h
> +++ b/include/uapi/sound/asound.h
> @@ -801,7 +801,14 @@ enum {
> 
>  struct snd_timer_tread {
>         int event;
> +#if __BITS_PER_LONG == 32
> +       struct {
> +               __kernel_long_t tv_sec;
> +               __kernel_long_t tv_usec;
> +       };
> +#else
>         struct timespec tstamp;
> +#endif
>         unsigned int val;
>  };
> 
> This has a somewhat higher risk of breaking existing code (since the type
> changes), and it doesn't solve the overflow.

Hm, how to define the timestamp type is one of the biggest questions
indeed.  In general, there can't be any guarantee that just rebuilding
with the 64bit timespec would work for all existing codes.  In theory
it shouldn't break, but who knows...


thanks,

Takashi

Re: [alsa-devel] [RFC PATCH v2 7/7] sound: core: Avoid using timespec for struct snd_timer_tread

[Arnd Bergmann]

On Sun, Nov 5, 2017 at 5:59 PM, Takashi Iwai <tiwai@suse.de> wrote:
> On Sun, 05 Nov 2017 14:16:28 +0100,
>> On Sun, Nov 5, 2017 at 11:29 AM, Takashi Iwai <tiwai@suse.de> wrote:
>> > On Thu, 02 Nov 2017 12:06:57 +0100,
>> > We should introduce SNDRV_TIMER_IOCTL_USER_PVERSION instead, where
>> > user-space can tell which protocol version it understands.  If the
>> > protocol version is higher than some definition, we can assume it's
>> > 64-bit ready.  The *_USER_PVERSION is issued from alsa-lib side.
>> > In that way, we can extend the ABI more flexibly.  A similar trick was
>> > already used in PCM ABI.  (Ditto for control and rawmidi API; we
>> > should have the same mechanism for all relevant APIs).
>> >
>> > Moreover, once when the new protocol is used, we can use the standard
>> > 64bit monotonic nsecs as a timestamp, so that we don't need to care
>> > about 32/64bit compatibility.
>>
>> I think that's fine, we can do that too, but I don't see how we get around
>> to doing something like Baolin's patch first. Without this, we will get
>> existing user space code compiling against our kernel headers using a
>> new glibc release that will inadvertently change the structure layout
>> on the read file descriptor.
>
> But it won't work in anyway in multiple ways, e.g. this timer read
> stuff and another the structs embedded in the mmappged page.  If you
> do rebuild things with new glibc, it should tell kernel about the new
> ABI in anyway more or less explicitly.  And if you need it, it means
> that some source-code level API change would be possible.

Right, you mentioned the mmap interface at the kernel summit. This
is certainly the most tricky part and will probably require source-level
changes.

Can you clarify a few things about the mmap() interface?
Is this specifically about "struct snd_pcm_mmap_status" on the
pcm device, or are there others?

>From what I can see, it's already fairly limited:
- on most architectures, it's completely disabled, only x86, ppc and
  alpha allow it to start with, and user space can work around
  the mmap not being available by falling back to ioctl if I read
  the comments correctly
- alpha is not affected since time_t is always 64-bit
- x86 and ppc disable the mmap() in compat mode already because
  of the same issue. If it comes to the worst, we can probably do
  the same for x86-32 and ppc32, disabling the existing status mmap
  for them as well, and change SNDRV_PCM_MMAP_OFFSET_STATUS
  to a new value for 32-bit kernels that exposes the same structure
  as 64-bit kernels.
- I think that since we always use an offset that is defined in the
  header file, we can use the same trick for mmap that we have
  for the ioctl command number:

diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
index c227ccba60ae..bcdbdac097d9 100644
--- a/include/uapi/sound/asound.h
+++ b/include/uapi/sound/asound.h
@@ -306,10 +306,19 @@ typedef int __bitwise snd_pcm_state_t;

 enum {
        SNDRV_PCM_MMAP_OFFSET_DATA = 0x00000000,
-       SNDRV_PCM_MMAP_OFFSET_STATUS = 0x80000000,
+       SNDRV_PCM_MMAP_OFFSET_STATUS_OLD = 0x80000000,
        SNDRV_PCM_MMAP_OFFSET_CONTROL = 0x81000000,
+       SNDRV_PCM_MMAP_OFFSET_STATUS64 = 0x82000000,
 };

+#if __BITS_PER_LONG == 64
+#define SNDRV_PCM_MMAP_OFFSET_STATUS SNDRV_PCM_MMAP_OFFSET_STATUS_OLD
+#else
+#define SNDRV_PCM_MMAP_OFFSET_STATUS ((sizeof(time_t) >
sizeof(__kernel_long_t)) ? \
+                                       SNDRV_PCM_MMAP_OFFSET_STATUS64 : \
+                                       SNDRV_PCM_MMAP_OFFSET_STATUS_OLD)
+#endif
+
 union snd_pcm_sync_id {
        unsigned char id[16];
        unsigned short id16[8];

Does that make sense?

> Of course, passing which data type is another question.  Maybe 64bit
> nsecs wouldn't fit all places, and timespec64 style would be still
> required.  But still, the current patch looks still too unnecessarily
> complex to me.  (Yeah I know that the problem is complex, but the code
> can be simpler, I hope!)

I think we can simplify the x86_32 case, but probably not much beyond
that. The trick above however can fix 32-bit compat mode for mmap
if we want to do that.

>> The trick with redefining SNDRV_TIMER_IOCTL_TREAD in that
>> configuration lets the kernel know what API the user space expects
>> without requiring source-level changes.
>
> Right, it works for this case, but not always.
> If jumping the API would give a cleaner way of implementation, I'd
> prefer that over too hackeries, IMO.

Generally speaking I try to avoid being incompatible since that causes
more problems for users when they either fail to build existing source
code, or get silent interface breakage after recompiling against a new
glibc. If the kernel can make it work, that should be the first priority.

>> If you want to for all users of SNDRV_TIMER_IOCTL_TREAD to move
>> to a new interface for y2038-safety, we'd have to redefined the structure
>> to avoid the libc-provided 'struct timespec' on 32-bit architectures, like:
>>
>> diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
>> index 299a822d2c4e..f93cace4cd24 100644
>> --- a/include/uapi/sound/asound.h
>> +++ b/include/uapi/sound/asound.h
>> @@ -801,7 +801,14 @@ enum {
>>
>>  struct snd_timer_tread {
>>         int event;
>> +#if __BITS_PER_LONG == 32
>> +       struct {
>> +               __kernel_long_t tv_sec;
>> +               __kernel_long_t tv_usec;
>> +       };
>> +#else
>>         struct timespec tstamp;
>> +#endif
>>         unsigned int val;
>>  };
>>
>> This has a somewhat higher risk of breaking existing code (since the type
>> changes), and it doesn't solve the overflow.
>
> Hm, how to define the timestamp type is one of the biggest questions
> indeed.  In general, there can't be any guarantee that just rebuilding
> with the 64bit timespec would work for all existing codes.  In theory
> it shouldn't break, but who knows...

Right we can capture most cases then user space gets the kernel
headers from /usr/include/linux and that gets created from a new enough
kernel, or when the ioctl command number is defined in terms of
the variable structure sizes that actually differ. This covers almost all
interfaces, but I've seen some exceptions that will be silently broken
no matter what we do. For all I can see, ALSA ioctls are fine, it's just a lot
of work to get right. The mmap() problem might be fairly easy to solve
in the end, or it may be very hard.

      Arnd

Re: [alsa-devel] [RFC PATCH v2 7/7] sound: core: Avoid using timespec for struct snd_timer_tread

[Takashi Iwai]

On Mon, 06 Nov 2017 17:33:26 +0100,
Arnd Bergmann wrote:
> 
> On Sun, Nov 5, 2017 at 5:59 PM, Takashi Iwai <tiwai@suse.de> wrote:
> > On Sun, 05 Nov 2017 14:16:28 +0100,
> >> On Sun, Nov 5, 2017 at 11:29 AM, Takashi Iwai <tiwai@suse.de> wrote:
> >> > On Thu, 02 Nov 2017 12:06:57 +0100,
> >> > We should introduce SNDRV_TIMER_IOCTL_USER_PVERSION instead, where
> >> > user-space can tell which protocol version it understands.  If the
> >> > protocol version is higher than some definition, we can assume it's
> >> > 64-bit ready.  The *_USER_PVERSION is issued from alsa-lib side.
> >> > In that way, we can extend the ABI more flexibly.  A similar trick was
> >> > already used in PCM ABI.  (Ditto for control and rawmidi API; we
> >> > should have the same mechanism for all relevant APIs).
> >> >
> >> > Moreover, once when the new protocol is used, we can use the standard
> >> > 64bit monotonic nsecs as a timestamp, so that we don't need to care
> >> > about 32/64bit compatibility.
> >>
> >> I think that's fine, we can do that too, but I don't see how we get around
> >> to doing something like Baolin's patch first. Without this, we will get
> >> existing user space code compiling against our kernel headers using a
> >> new glibc release that will inadvertently change the structure layout
> >> on the read file descriptor.
> >
> > But it won't work in anyway in multiple ways, e.g. this timer read
> > stuff and another the structs embedded in the mmappged page.  If you
> > do rebuild things with new glibc, it should tell kernel about the new
> > ABI in anyway more or less explicitly.  And if you need it, it means
> > that some source-code level API change would be possible.
> 
> Right, you mentioned the mmap interface at the kernel summit. This
> is certainly the most tricky part and will probably require source-level
> changes.
> 
> Can you clarify a few things about the mmap() interface?
> Is this specifically about "struct snd_pcm_mmap_status" on the
> pcm device, or are there others?
> 
> From what I can see, it's already fairly limited:
> - on most architectures, it's completely disabled, only x86, ppc and
>   alpha allow it to start with, and user space can work around
>   the mmap not being available by falling back to ioctl if I read
>   the comments correctly
> - alpha is not affected since time_t is always 64-bit
> - x86 and ppc disable the mmap() in compat mode already because
>   of the same issue. If it comes to the worst, we can probably do
>   the same for x86-32 and ppc32, disabling the existing status mmap
>   for them as well, and change SNDRV_PCM_MMAP_OFFSET_STATUS
>   to a new value for 32-bit kernels that exposes the same structure
>   as 64-bit kernels.
> - I think that since we always use an offset that is defined in the
>   header file, we can use the same trick for mmap that we have
>   for the ioctl command number:
> 
> diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
> index c227ccba60ae..bcdbdac097d9 100644
> --- a/include/uapi/sound/asound.h
> +++ b/include/uapi/sound/asound.h
> @@ -306,10 +306,19 @@ typedef int __bitwise snd_pcm_state_t;
> 
>  enum {
>         SNDRV_PCM_MMAP_OFFSET_DATA = 0x00000000,
> -       SNDRV_PCM_MMAP_OFFSET_STATUS = 0x80000000,
> +       SNDRV_PCM_MMAP_OFFSET_STATUS_OLD = 0x80000000,
>         SNDRV_PCM_MMAP_OFFSET_CONTROL = 0x81000000,
> +       SNDRV_PCM_MMAP_OFFSET_STATUS64 = 0x82000000,
>  };
> 
> +#if __BITS_PER_LONG == 64
> +#define SNDRV_PCM_MMAP_OFFSET_STATUS SNDRV_PCM_MMAP_OFFSET_STATUS_OLD
> +#else
> +#define SNDRV_PCM_MMAP_OFFSET_STATUS ((sizeof(time_t) >
> sizeof(__kernel_long_t)) ? \
> +                                       SNDRV_PCM_MMAP_OFFSET_STATUS64 : \
> +                                       SNDRV_PCM_MMAP_OFFSET_STATUS_OLD)
> +#endif
> +
>  union snd_pcm_sync_id {
>         unsigned char id[16];
>         unsigned short id16[8];
> 
> Does that make sense?

Yeah, that should work.

But can we make the flip without the dynamic sizeof() comparison but
some ifdef?  The above doesn't allow the usage with switch(), for
example.

IOW, is there any macro indicating the 64bit user time_t?

In theory we can have the shadow mmap for the compat timespec, and
convert it always when the status gets changed.  But I guess disabling
the mmap should work simply as is, judging from the 64bit compat
status.


So, basically speaking, I find all fine with the suggested
conversions.  But, some details look fairly ugly, like the dynamic
const evaluation in the above.  If we can tidy up these devils, the
code will become more readable.


thanks,

Takashi

Re: [alsa-devel] [RFC PATCH v2 7/7] sound: core: Avoid using timespec for struct snd_timer_tread

[Arnd Bergmann]

On Thu, Nov 9, 2017 at 5:52 PM, Takashi Iwai <tiwai@suse.de> wrote:
> On Mon, 06 Nov 2017 17:33:26 +0100,

>> --- a/include/uapi/sound/asound.h
>> +++ b/include/uapi/sound/asound.h
>> @@ -306,10 +306,19 @@ typedef int __bitwise snd_pcm_state_t;
>>
>>  enum {
>>         SNDRV_PCM_MMAP_OFFSET_DATA = 0x00000000,
>> -       SNDRV_PCM_MMAP_OFFSET_STATUS = 0x80000000,
>> +       SNDRV_PCM_MMAP_OFFSET_STATUS_OLD = 0x80000000,
>>         SNDRV_PCM_MMAP_OFFSET_CONTROL = 0x81000000,
>> +       SNDRV_PCM_MMAP_OFFSET_STATUS64 = 0x82000000,
>>  };
>>
>> +#if __BITS_PER_LONG == 64
>> +#define SNDRV_PCM_MMAP_OFFSET_STATUS SNDRV_PCM_MMAP_OFFSET_STATUS_OLD
>> +#else
>> +#define SNDRV_PCM_MMAP_OFFSET_STATUS ((sizeof(time_t) >
>> sizeof(__kernel_long_t)) ? \
>> +                                       SNDRV_PCM_MMAP_OFFSET_STATUS64 : \
>> +                                       SNDRV_PCM_MMAP_OFFSET_STATUS_OLD)
>> +#endif
>> +
>>  union snd_pcm_sync_id {
>>         unsigned char id[16];
>>         unsigned short id16[8];
>>
>> Does that make sense?
>
> Yeah, that should work.
>
> But can we make the flip without the dynamic sizeof() comparison but
> some ifdef?  The above doesn't allow the usage with switch(), for
> example.
>
> IOW, is there any macro indicating the 64bit user time_t?

There is a macro defined by the C library, but so far we have not
started relying on it in kernel headers, because there is no guarantee
that this symbol is visible before sys/time.h has been included,
and there are some cases where it's possible to include a kernel
header before sys/time.h.

In case of sound/asound.h, that should be no problem since we rely
on having seen the definition on 'struct timeval' already today, and
that must come from sys/time.h. Then we just need to make sure that
all C libraries define the same macro.

Are you sure about the switch()/case problem? I thought that worked
in C99, the only problem would be using the macro outside of a
function, e.g. as initalizer for a variable

> In theory we can have the shadow mmap for the compat timespec, and
> convert it always when the status gets changed.  But I guess disabling
> the mmap should work simply as is, judging from the 64bit compat
> status.

Ok.

       Arnd

Re: [alsa-devel] [RFC PATCH v2 7/7] sound: core: Avoid using timespec for struct snd_timer_tread

[Takashi Iwai]

On Thu, 09 Nov 2017 18:01:47 +0100,
Arnd Bergmann wrote:
> 
> On Thu, Nov 9, 2017 at 5:52 PM, Takashi Iwai <tiwai@suse.de> wrote:
> > On Mon, 06 Nov 2017 17:33:26 +0100,
> 
> >> --- a/include/uapi/sound/asound.h
> >> +++ b/include/uapi/sound/asound.h
> >> @@ -306,10 +306,19 @@ typedef int __bitwise snd_pcm_state_t;
> >>
> >>  enum {
> >>         SNDRV_PCM_MMAP_OFFSET_DATA = 0x00000000,
> >> -       SNDRV_PCM_MMAP_OFFSET_STATUS = 0x80000000,
> >> +       SNDRV_PCM_MMAP_OFFSET_STATUS_OLD = 0x80000000,
> >>         SNDRV_PCM_MMAP_OFFSET_CONTROL = 0x81000000,
> >> +       SNDRV_PCM_MMAP_OFFSET_STATUS64 = 0x82000000,
> >>  };
> >>
> >> +#if __BITS_PER_LONG == 64
> >> +#define SNDRV_PCM_MMAP_OFFSET_STATUS SNDRV_PCM_MMAP_OFFSET_STATUS_OLD
> >> +#else
> >> +#define SNDRV_PCM_MMAP_OFFSET_STATUS ((sizeof(time_t) >
> >> sizeof(__kernel_long_t)) ? \
> >> +                                       SNDRV_PCM_MMAP_OFFSET_STATUS64 : \
> >> +                                       SNDRV_PCM_MMAP_OFFSET_STATUS_OLD)
> >> +#endif
> >> +
> >>  union snd_pcm_sync_id {
> >>         unsigned char id[16];
> >>         unsigned short id16[8];
> >>
> >> Does that make sense?
> >
> > Yeah, that should work.
> >
> > But can we make the flip without the dynamic sizeof() comparison but
> > some ifdef?  The above doesn't allow the usage with switch(), for
> > example.
> >
> > IOW, is there any macro indicating the 64bit user time_t?
> 
> There is a macro defined by the C library, but so far we have not
> started relying on it in kernel headers, because there is no guarantee
> that this symbol is visible before sys/time.h has been included,
> and there are some cases where it's possible to include a kernel
> header before sys/time.h.
> 
> In case of sound/asound.h, that should be no problem since we rely
> on having seen the definition on 'struct timeval' already today, and
> that must come from sys/time.h. Then we just need to make sure that
> all C libraries define the same macro.
> 
> Are you sure about the switch()/case problem? I thought that worked
> in C99, the only problem would be using the macro outside of a
> function, e.g. as initalizer for a variable

Hmm, OK it seems working.

But, honestly speaking, it's too scaring.  I'm OK if it were only in
the kernel local code.  But it's the API/ABI definition, which is
referred by user-space...

A more solid condition would be really appreciated.


thanks,

Takashi

Re: [alsa-devel] [RFC PATCH v2 7/7] sound: core: Avoid using timespec for struct snd_timer_tread

[Arnd Bergmann]

On Thu, Nov 9, 2017 at 7:11 PM, Takashi Iwai <tiwai@suse.de> wrote:
> On Thu, 09 Nov 2017 18:01:47 +0100,
> Arnd Bergmann wrote:
>>
>> On Thu, Nov 9, 2017 at 5:52 PM, Takashi Iwai <tiwai@suse.de> wrote:
>> >
>> > IOW, is there any macro indicating the 64bit user time_t?
>>
>> There is a macro defined by the C library, but so far we have not
>> started relying on it in kernel headers, because there is no guarantee
>> that this symbol is visible before sys/time.h has been included,
>> and there are some cases where it's possible to include a kernel
>> header before sys/time.h.
>>
>> In case of sound/asound.h, that should be no problem since we rely
>> on having seen the definition on 'struct timeval' already today, and
>> that must come from sys/time.h. Then we just need to make sure that
>> all C libraries define the same macro.
>>
>> Are you sure about the switch()/case problem? I thought that worked
>> in C99, the only problem would be using the macro outside of a
>> function, e.g. as initalizer for a variable
>
> Hmm, OK it seems working.
>
> But, honestly speaking, it's too scaring.  I'm OK if it were only in
> the kernel local code.  But it's the API/ABI definition, which is
> referred by user-space...
>
> A more solid condition would be really appreciated.

I understand your concern here and agree it's really ugly. It did take us
many attempts to come up with this trick for other cases, so my initial
reaction would be to use the same thing everywhere since I know
it works,  but we can use #ifdef instead if you prefer that. I think we
can use a single #ifdef variant to cover all cases, but I'd have to think
about the x32 and x86-32 some more here. With this trick, we can
make user space with new glibc use data structures that are compatible
with 64-bit kernels and avoid the additional translation helpers:

enum {
      SNDRV_PCM_MMAP_OFFSET_DATA = 0x00000000,
      SNDRV_PCM_MMAP_OFFSET_CONTROL = 0x81000000,
#if (__BITS_PER_LONG == 64) || !defined(__USE_TIME_BITS64)
      SNDRV_PCM_MMAP_OFFSET_STATUS = 0x80000000,
#else
      /* New snd_pcm_mmap_status layout on 32-bit architectures */
      SNDRV_PCM_MMAP_OFFSET_STATUS = 0x82000000,
#endif
};

struct snd_pcm_mmap_status {
        snd_pcm_state_t state;          /* RO: state - SNDRV_PCM_STATE_XXXX */
        int pad1;                                 /* Needed for 64 bit
alignment */
#if (__BITS_PER_LONG == 32 && defined(__USE_TIME_BITS64)
        __u64 hw_ptr;                        /* for compatibility with
64-bit layout */
#else
       snd_pcm_uframes_t hw_ptr;   /* RO: hw ptr (0...boundary-1) */
#endif
        struct timespec tstamp;          /* Timestamp */
        snd_pcm_state_t suspended_state; /* RO: suspended stream state */
#if (__BITS_PER_LONG == 32 && defined(__USE_TIME_BITS64)
        int pad2;
#endif
        struct timespec audio_tstamp;   /* from sample counter or wall clock */
};
#endif

struct snd_pcm_mmap_control {
#if (__BITS_PER_LONG == 32 && defined(__USE_TIME_BITS64)
       __u64 appl_ptr;     /* RW: appl ptr (0...boundary-1) */
        __u64 avail_min;    /* RW: min available frames for wakeup */
#else
        snd_pcm_uframes_t appl_ptr;     /* RW: appl ptr (0...boundary-1) */
        snd_pcm_uframes_t avail_min;    /* RW: min available frames
for wakeup */
#endif
};

struct snd_pcm_sync_ptr {
        unsigned int flags;
#if (__BITS_PER_LONG == 32 && defined(__USE_TIME_BITS64)
        __u32 __pad; /* for x86-32 */
#endif
       union {
                struct snd_pcm_mmap_status status;
                unsigned char reserved[64];
        } s;
        union {
                struct snd_pcm_mmap_control control;
                unsigned char reserved[64];
        } c;
};

struct snd_timer_tread {
        int event;
#if (__BITS_PER_LONG == 32) && defined(__USE_TIME_BITS64)
        int __pad0; /* make alignment sane on x86_32 */
#endif
       struct timespec tstamp;
        unsigned int val;
#if (__BITS_PER_LONG == 32) && defined(__USE_TIME_BITS64)
        int __pad1;
#endif
};

/* note: on x32, the number will change with a new glibc, but the
behavior will not */
#if (__BITS_PER_LONG == 32) && defined(__USE_TIME_BITS64)
#define SNDRV_TIMER_IOCTL_TREAD         _IOW('T', 0x62, int)
#else
#define SNDRV_TIMER_IOCTL_TREAD         _IOW('T', 0x02, int)
#endif

Re: [alsa-devel] [RFC PATCH v2 7/7] sound: core: Avoid using timespec for struct snd_timer_tread

[Takashi Iwai]

On Fri, 10 Nov 2017 00:20:10 +0100,
Arnd Bergmann wrote:
> 
> On Thu, Nov 9, 2017 at 7:11 PM, Takashi Iwai <tiwai@suse.de> wrote:
> > On Thu, 09 Nov 2017 18:01:47 +0100,
> > Arnd Bergmann wrote:
> >>
> >> On Thu, Nov 9, 2017 at 5:52 PM, Takashi Iwai <tiwai@suse.de> wrote:
> >> >
> >> > IOW, is there any macro indicating the 64bit user time_t?
> >>
> >> There is a macro defined by the C library, but so far we have not
> >> started relying on it in kernel headers, because there is no guarantee
> >> that this symbol is visible before sys/time.h has been included,
> >> and there are some cases where it's possible to include a kernel
> >> header before sys/time.h.
> >>
> >> In case of sound/asound.h, that should be no problem since we rely
> >> on having seen the definition on 'struct timeval' already today, and
> >> that must come from sys/time.h. Then we just need to make sure that
> >> all C libraries define the same macro.
> >>
> >> Are you sure about the switch()/case problem? I thought that worked
> >> in C99, the only problem would be using the macro outside of a
> >> function, e.g. as initalizer for a variable
> >
> > Hmm, OK it seems working.
> >
> > But, honestly speaking, it's too scaring.  I'm OK if it were only in
> > the kernel local code.  But it's the API/ABI definition, which is
> > referred by user-space...
> >
> > A more solid condition would be really appreciated.
> 
> I understand your concern here and agree it's really ugly. It did take us
> many attempts to come up with this trick for other cases, so my initial
> reaction would be to use the same thing everywhere since I know
> it works,  but we can use #ifdef instead if you prefer that. I think we
> can use a single #ifdef variant to cover all cases, but I'd have to think
> about the x32 and x86-32 some more here. With this trick, we can
> make user space with new glibc use data structures that are compatible
> with 64-bit kernels and avoid the additional translation helpers:
> 
> enum {
>       SNDRV_PCM_MMAP_OFFSET_DATA = 0x00000000,
>       SNDRV_PCM_MMAP_OFFSET_CONTROL = 0x81000000,
> #if (__BITS_PER_LONG == 64) || !defined(__USE_TIME_BITS64)

Yeah, it's definitely better, more understandable!


thanks,

Takashi